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

Revisit buffer ownership in HttpData interface #18

Open
pderop opened this issue Oct 28, 2022 · 0 comments
Open

Revisit buffer ownership in HttpData interface #18

pderop opened this issue Oct 28, 2022 · 0 comments
Labels
enhancement New feature or request
Milestone

Comments

@pderop
Copy link
Collaborator

pderop commented Oct 28, 2022

in PR #17, an attempt has been made in order to clarify buffer ownership when you need to obtain the buffer from an HttpData.

Before PR #17, we were using two methods in order to get buffers from HttpData:

  • getBuffer() throws Exception
  • content()

So, for disk based data, a newly allocated buffer was returned by the getBuffer()/getContent(), while for memory based http data, the internal held buffer was returned.

So, the PR #17 did an attempt to eliminate the differences between the behaviors, in order to avoid confusing users: should they release or not the returned buffer. So, a new usingBuffer method has been introduced for replacement of the old getBuffer()/content() methods: it takes a lambda which accepts the http data buffer, and for disk based http data, then the passed buffer is immediately closed once the lambda callback returns.

This works greatly for the codec-multipart implementation, but it is actually questionable to have replaced the old public getBuffer()/content() method by the new "usingBuffer" method, because users may still corrupt the internal buffer in case the http data is memory based. Also, it's not practical to use a lambda in case you need to return the buffer to the caller.

So, maybe the usingBuffer method should be moved outside of the public API and remain private in the codec implementation, and something else should be done in order to clarify buffer ownership.

@pderop pderop added the enhancement New feature or request label Oct 28, 2022
@pderop pderop added this to the 5.0.0.Alpha2 milestone Oct 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant