search

FasterXML / jackson-core

Core part of Jackson that defines Streaming API as well as basic shared abstractions
Apache License 2.0
2.26k stars 792 forks source link

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

Closed cowtowncoder closed 6 years ago

cowtowncoder commented 6 years ago

(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 commented 6 years ago

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 commented 6 years ago

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