-
-
Notifications
You must be signed in to change notification settings - Fork 816
Description
(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.