If I understand correctly, the documentation does imply that such
synchronization at [2] should not be necessary and that [4] and [5] are
correct. Of course it would be better if the docs said _how_ this guarantee
is provided, or perhaps more than that it is a matter of restating it as a
requirement for any consumer of a DeserializationContext.

That said, if there's a bug causing the DeserializationContext to be
accessed concurrently, then working around it with additional
synchronization seems reasonable since a ValueProvider should be
deserialized infrequently (not on the critical data path). Do you know how
the multithreaded access is occurring?

Kenn

On Fri, Jan 28, 2022 at 7:30 AM Kerry Donny-Clark <[email protected]>
wrote:

> Hi Kellen, thanks for figuring this out. Have you implemented
> synchronizing on ctxt, or do you need someone else to try that?
>
> On Fri, Jan 28, 2022 at 12:36 AM Kellen Dye <[email protected]> wrote:
>
>> We (Spotify) are experiencing flaky tests on beam 2.35.0 as a result of
>> NullPointerExceptions during pipeline construction.
>>
>> Stacktrace [1]. Root cause appears to be multiple threads
>> accessing DeserializationContext from
>> ValueProvider.Deserializer.deserialize [2] which, according to the javadocs
>> "is guaranteed to only be used from single-threaded context" [3]
>>
>> In the context internals, the mutable _currentType is set [4] then
>> accessed a few lines later [5], but in a multithreaded situation the writes
>> clobber each other and a call to next() can fail with a NPE.
>>
>> I think synchronizing on ctxt in [2] should be sufficient to avoid this
>> issue, but I'm not sure if the NPEs are due to beam misuse or something on
>> the jackson side that should be changed.
>>
>> Failing (scala) test here, scalatest runs tests in parallel:
>>
>> https://gist.github.com/kellen/124185463c16a66167a7fa704147c510#file-fail-scala
>>
>> Test stacktrace:
>>
>> https://gist.github.com/kellen/124185463c16a66167a7fa704147c510#file-test_stacktrace-txt
>>
>> Cheers,
>> Kellen
>>
>> [1]
>> https://gist.github.com/kellen/124185463c16a66167a7fa704147c510#file-stacktrace-txt
>> [2]
>> https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ValueProvider.java#L359
>> [3]
>> https://github.com/joansmith/jackson-databind/blob/master/src/main/java/com/fasterxml/jackson/databind/DeserializationContext.java#L36
>>  [4]
>> https://github.com/joansmith/jackson-databind/blob/master/src/main/java/com/fasterxml/jackson/databind/DeserializationContext.java#L652
>> [5]
>> https://github.com/joansmith/jackson-databind/blob/master/src/main/java/com/fasterxml/jackson/databind/DeserializationContext.java#L656
>>
>>
>>

Reply via email to