Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dagwriter: fix reuse of buffers problem. #243

Closed
wants to merge 1 commit into from
Closed

Conversation

jbenet
Copy link
Member

@jbenet jbenet commented Oct 30, 2014

The buffers from the client were getting reused (not copied)
and so the data got duplicated internally. The tests don't
catch this!! I tested manually by adding a large text (aeneid).
We should make a test for it.

And, FWIW, tried making the channel sync (i.e. dagwriter
splChan not be buffered), which may be the right way to fix
the problem, but didnt work.

The buffers from the client were getting reused (not copied)
and so the data got duplicated internally. The tests don't
catch this!! I tested manually by adding a large text (aeneid).
We should make a test for it.

And, FWIW, tried making the channel sync (i.e. dagwriter
splChan not be buffered), which may be the right way to fix
the problem, but didnt work.
@jbenet jbenet added the status/in-progress In progress label Oct 30, 2014
@jbenet
Copy link
Member Author

jbenet commented Oct 30, 2014

I'd be ok including the aeneid as a test file: http://classics.mit.edu/Virgil/aeneid.mb.txt

@btc
Copy link
Contributor

btc commented Oct 30, 2014

That'll come in handy.

@jbenet
Copy link
Member Author

jbenet commented Oct 31, 2014

Closed in favor of #244

@jbenet jbenet closed this Oct 31, 2014
@jbenet jbenet removed the status/in-progress In progress label Oct 31, 2014
@Kubuxu Kubuxu deleted the fix-dagwriter branch February 27, 2017 20:39
@aschmahmann aschmahmann mentioned this pull request Dec 1, 2021
80 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants