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

Problem with BufferRecycler via async parser (or when sharing parser across threads) #476

Closed
cowtowncoder opened this issue Sep 6, 2018 · 2 comments

Comments

@cowtowncoder
Copy link
Member

(note: came from https://jira.spring.io/browse/SPR-17193)

It seems that ThreadLocal-based scheme used via BufferRecycler/BufferRecyclers does not work reliably if JsonParser (and probably JsonGenerator too) instance is used from more than one thread. This is commonly case for non-blocking parsing.
The problem is likely due to the fact that parser/generator obtains a BufferRecycler container from ThreadLocal and retains it during whole operation: and although underlying buffer references are cleared (to avoid accidental sharing), those operation are assumed to come from a single thread and are not synchronized.

There are multiple ways we could approach fixing. For example, it would be possible to change buffer references to use AtomicReference, or use equivalent data structure (like AtomicReferenceArray) to contain references. That would add synchronization overhead for individual access; this might not be a big problem.

Alternatively we could see if the whole buffer recycling should be disabled for async parser: this is probably feasible thing to do by simply always constructing clean BufferRecycler for new parsers.
This would not help possible case of accessing a parser/generator from multiple threads (like separate initial processor, passing to a worker thread), however. It may or may not have worse performance as well, as no reuse would occur.

But perhaps the most interesting approach would be to instead guard access to BufferRecycler behind AtomicReference (or alternatively add AtomicBoolean marker in recycler, as a mark of "in use"). This would add overhead in just two places: when obtaining and when releasing instance.
Unfortunately there is one significant problem: as things are, there is no place where recycler instance is "returned"; and this is by design as there is no easy place to do it from.
It may be possible to find a place, however, since parsers do have mechanism for returning individual buffers -- this would seem like the place for "releasing" recycler itself too.

cowtowncoder added a commit that referenced this issue Sep 11, 2018
@cowtowncoder
Copy link
Member Author

Was able to reproduce this with a unit test which can be made to pass by disabling buffer recycling. Hope to work on solution soon.

@cowtowncoder
Copy link
Member Author

2.9.7 is out now: https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.9.7

sdeleuze added a commit to sdeleuze/spring-framework that referenced this issue Aug 28, 2019
Since now FasterXML/jackson-core#476
and FasterXML/jackson-core#479 are fixed.

This commit also raises the minimum version of Jackson to 2.9.7.

Closes spring-projectsgh-23522
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

No branches or pull requests

1 participant