Bugzilla – Full Text Bug Listing
| Summary: | Wrong reuse of buffers in CachedExchange | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [RT] Jetty | Reporter: | Mathieu Poumeyrol <kali> | ||||||||
| Component: | client | Assignee: | Greg Wilkins <gregw> | ||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||
| Severity: | normal | ||||||||||
| Priority: | P3 | CC: | jesse.mcconnell, jetty-inbox | ||||||||
| Version: | 7.2.1 | ||||||||||
| Target Milestone: | 7.2.x | ||||||||||
| Hardware: | Macintosh | ||||||||||
| OS: | Mac OS X - Carbon (unsup.) | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Mathieu Poumeyrol
Created attachment 186770 [details]
test case
*** Bug 334309 has been marked as a duplicate of this bug. *** *** Bug 334310 has been marked as a duplicate of this bug. *** Created attachment 186810 [details]
junitized test case
In the current state of the code, both assertion in testHeaderWhenReadEarly pass, whereas the assert() testHeaderWhenReadLate breaks.
It appears that the bug does not appear when headers are read before the second exchange is triggered.
I am not sure wether or not calling getResponseFields() in onResponseComplete() warrants the headers values will be cached *before* another exchange use the shared buffers again when execution order is not imposed.
Created attachment 186814 [details]
patch
For some reason I don't fully understand, code in HttpFields makes deep copy of headers names, but only build View for "_value" buffer. The proposed patch makes a deep copy of the value calling asImmutableBuffer() before passing it to HttpFields in onResponsePatch(). It make the test case pass and seams quite innocuous. As a matter of fact, for the sake of concern isolation and consistency, I think the header name should be passed as an immutable buffer too. Current code in HttpFields.add() will make it immutable anyway so there should be no performance impact at all. hm, thanks for this will take a deeper look soon Sending VERSION.txt Sending jetty-client/src/main/java/org/eclipse/jetty/client/CachedExchange.java Adding jetty-client/src/test/java/org/eclipse/jetty/client/CachedHeadersIsolationTest.java Transmitting file data ... Committed revision 2663. I have committed this fix but before I close this issue I am going to assign to Greg for some additional review. thanks |