Evgeny Kotkov <[email protected]> writes: > Luckily, in this particular case with HPACK buckets and header-only > requests, it's possible to avoid using the request->allocator, and > allocate the new bucket using the HTTP/2 stream's allocator. Thus, > the lifetime of the new bucket will no longer depend on the underlying > serf_request_t. > > Please see the attached patch with the fix described above. The log > message is included in the beginning of the patch file.
After giving it a fresh look, I think that there is a downside in the proposed fix. While it solves the issue, currently the HTTP/2 stream's allocator is set to conn->allocator. Thus, with the fix the per-request HPACK buckets would be allocated using the per-connection allocator. That's error-prone, since such buckets could not be destroyed in some edge cases (perhaps, during errors or when a request is cancelled), and that could result in a memory leak. With this in mind, I would like to think about it for some time. Regards, Evgeny Kotkov
