Concurrency issues on the cached Jsonb instance

[imgea]

Describe the bug
We have a cached static Jsonb instance used application wide and only with the latest release (23.0.0.10) we started running into concurrency issues with multiple threads executing fromJson & toJson methods.
For example, we can observe malformed json outputs as a result of toJson method.

Steps to Reproduce
Using a cached Jsonb application wide under high load

  private static final Jsonb jsonb = JsonbBuilder.newBuilder().withConfig(
    								   new JsonbConfig().withAdapters(new AtomicLongAdapter(),new AtomicIntegerAdapter())).build();

Expected behavior
The usage of the cached Jsonb instance's methods should be thread-safe as with the previous OL versions.

Diagnostic information:

• OpenLiberty Version: 23.0.0.10
• Affected feature(s) [e.g. jsonb-3.0]
• Java Version:

 openjdk version "17.0.8.1" 2023-08-24
IBM Semeru Runtime Open Edition 17.0.8.1 (build 17.0.8.1+1)
Eclipse OpenJ9 VM 17.0.8.1 (build openj9-0.40.0, JRE 17 Linux amd64-64-Bit Compressed References 20230824_549 (JIT enabled, AOT enabled)
OpenJ9   - d12d10c9e
OMR      - e80bff83b
JCL      - 8ecf238a124 based on jdk-17.0.8.1+1)

• server.xml:

 <server description="Server">
    <featureManager>
        <feature>microProfile-6.0</feature>
	<feature>servlet-6.0</feature>
    </featureManager>
    <httpEndpoint httpPort="8080" httpsPort="8443" id="defaultHttpEndpoint"  host="*" />
    <webApplication location="app.war" contextRoot="/" />
</server>

Additional context
When we enable jsonbContainer-3.0 feature and reference yasson libraries (yasson-3.0.0.jar,..,yasson-3.0.3.jar) like the below server.xml config then we encounter no concurrency issues with the cached Jsonb instance.

<featureManager>
        <feature>microProfile-6.0</feature>
	<feature>servlet-6.0</feature>	
    	<feature>jsonbContainer-3.0</feature> 
</featureManager>
<bell libraryRef="yassonLib"/>
<library id="yassonLib">
	<fileset dir="${server.config.dir}/yasson" includes="*.jar"/>
 </library> 

[jhanders34]

In 23.0.0.10, Open Liberty updated to Yasson 3.0.3 and Parsson 1.1.4. The update to the Yasson 3.0.3 caused similar issues to what are reported here and related to a defect in the Parsson code that required the update to Parsson. That problem was related to when things were closed in Parsson.

The Jsonb class javadoc states below.

All the methods in this class are safe for use by multiple concurrent threads.

Calling Closable.close() method will cleanup all CDI managed components (such as adapters with CDI 
dependencies) created during interaction with Jsonb. Calling close() must be done after all threads has 
finished interaction with Jsonb. If there are remaining threads working with Jsonb and close() is called, 
behaviour is undefined. 

In the scenario that you are attempting, do you ever close the Jsonb object either explicitly or with a try resources block?

[imgea]

No, in our case close() is not being called.

[KyleAure]

So here is a timeline from this yasson/parsson compatibility situation:

• When EE 10 released both implementations were compatible.
• Yasson introduced an update in version 3.0.3 which ensured that streams were closed upon success completion of toJson/fromJson methods.
  • Issue: JsonBinding#fromJson not closing InputStream after successfull completion eclipse-ee4j/yasson#591
  • Change: Close Parsers and Generators eclipse-ee4j/yasson#586
• This change was introduced to ensure that Yasson meet the expectation set in JSON-B's API:
  • (&)Streams: https://github.com/jakartaee/jsonb-api/blob/7340b84ec2a5bbdeb492d425eccee6d65443e97c/api/src/main/java/jakarta/json/bind/Jsonb.java#L219-L220
  • (*)Reader/Writer: https://github.com/jakartaee/jsonb-api/blob/7340b84ec2a5bbdeb492d425eccee6d65443e97c/api/src/main/java/jakarta/json/bind/Jsonb.java#L175-L176
• Now that Yasson is calling close on these objects a bug is being exploited in Parsson leading to malformed outputs. Resulting Parsson version 1.1.3 to be released to fix this issue:
  • Issue: JsonTokenizer.close() recycles its buffer for each call to close() which may case data corruption in a multi-threaded environment eclipse-ee4j/parsson#77
  • Change: #77: JsonTokenizer.close() recycles its buffer for each call to close() eclipse-ee4j/parsson#97
• Liberty updated to using Yasson 3.0.3 and Parsson 1.1.4 starting with release 23.0.0.10

Did you catch the issues?
(*) Based on the JSONB API only streams should be closed after successful process, not Readers/Writers.
So before 3.0.3 Yasson was failing to implement JSONB because it was not closing streams, but now it is failing to implement JSONB because it is closing reader/writers.
This is currently being discussed here: jakartaee/jsonb-api#346
To decide if the JSONB API should be updated to reflect what Yasson did, or to revert to not closing these objects in the first place.

(&) Yasson also placed the close in the finally block (using try-with-resources) which means it is closing streams even when there is a failure further failing to implement the JSONB API

What about Parsson?
Parsson's buffer implementation was updated to only recycle their buffer once instead of on each close.
Meaning that we should be avoiding malformed output no matter which Yasson version used.

Next Steps:
I'd be interested in doing some more investigation on the issue you are currently experiencing.
To my knowledge the yasson/parsson versions we are shipping should be compatible and not cause any concurrency issues.
The best coding practice there is to ensure that the resource created by your application is closed by your application:

try (var ioStream = createIoStream()) {
  jsonb.toJson(object, ioStream);
}

@imgea
Are you able to replicate this consistently?
Are you using streams or reader/writers?
What is your expected output vs actual output (obfuscated if necessary)?
Could you provide a snippet of code I could use to test?

[imgea]

We aren't using any streams or reader/writers. But digging into it more I've found that this concurrency errors happened only when we had a library introducing yasson library to our build path (as both compile & runtime artifact).
The concurrency issues persist regardless of the yasson version we have in our build path (happens on both yasson-3.0.2 & yasson-3.0.3). In our case we don't need to have yasson in the final artifact but if you'd like I can try preparing a repo that reproduces the issue.

[imgea]

Here you can find a small repo that reproduces the issue: https://github.com/imgea/ol-jsonb-test/

[KyleAure]

@imgea thanks for providing a reproducer for this.
I tested it out and it looks like the way this application is being built might be the issue.
If I take the application that is created using Gradle and decompile/unzip it I see the following directory layout:

It looks like yasson/parsson are being packaged with the application and the versions that are being packaged together are incompatible yasson-3.0.3 / parsson-1.1.0
This is likely caused by the fact that the yasson-3.0.3 artifact still lists a compile time dependency for parsson-1.1.0 instead parsson-1.1.3+

Your applications do not need to have yasson/parsson dependencies, those are provided for you by Liberty though our features. You should slim down your application by using the compileOnly configuration in gradle.

Therefore your original project configuration:

https://github.com/imgea/ol-jsonb-test/blob/77ef8798eedfba5993ec41a5e19506ba3c5d22da/build.gradle#L15-L26

would become:

dependencies {
    // Application Dependencies
    implementation 'org.slf4j:slf4j-api:1.7.30'
    implementation 'ch.qos.logback:logback-classic:1.4.11'
    implementation 'org.mongodb:bson:4.8.0'
    implementation 'com.google.guava:guava:31.0.1-jre'

    // Dependencies provided by Open Liberty
    compileOnly 'org.eclipse.microprofile:microprofile:6.0'
    compileOnly group: 'org.eclipse', name: 'yasson', version: '3.0.3'
}

I would suggest checking your production application(s) and verify that any Microprofile/JakartaEE artifacts are not being packaged in with your applications.

[imgea]

Yes indeed we've already fixed the library that introduced the yasson dependency.
It is clear now the issue was with having yasson-3.0.3 with parsson-1.1.0.
Thank you for the explanations and the support.

[inad9300]

@KyleAure I'm experiencing this very problem after migrating to 23.0.0.10. However, removing "yasson" as a direct dependency of my project only lead me to an error:

jakarta.json.bind.JsonbException: JSON Binding provider org.eclipse.yasson.JsonBindingProvider not found
Caused by: java.lang.ClassNotFoundException: org.eclipse.yasson.JsonBindingProvider

[inad9300]

My bad, I did not correctly interpret your suggestion at first. I got it working now with the following Maven configuration:

<dependency>
  <groupId>org.eclipse</groupId>
  <artifactId>yasson</artifactId>
  <version>3.0.3</version>
  <scope>provided</scope><!-- The important bit. -->
</dependency>

I'm still not sure and would like to understand why adding "provided" / "compileOnly" makes it work. Both with and without it, Yasson 3.0.3 shows up as a dependency, so what's different?

[jhanders34]

When you say provided or compileOnly in your maven configuration, the artifact is not placed into your application. You are saying I need this artifact in order to compile my application or run unit tests on it, but I do not need it in the application when it is deployed to an application server because the application server provides the function.

[inad9300]

I guess my question is, if Open Liberty already comes with a valid JSON-B implementation, why do I need Yasson as an additional dependency? It feels redundant.

[jhanders34]

I guess my question is, if Open Liberty already comes with a valid JSON-B implementation, why do I need Yasson as an additional dependency? It feels redundant.

You should not need that dependency at all. As mentioned in my earlier reply, you may have a unit test dependency in your maven project if you can run tests outside of an application server. If you do not have that requirement, then yes it can be removed.

[inad9300]

It took me a while to get to validate this, but I confirm I could indeed remove the dependency, and it was only the tests that would fail. In fact, <scope>test</scope> works too – which now has got me wondering what scope would be best to set.

[KyleAure]

Likely the reason your test scope worked is because your src code does not actually use any implementation classes from yasson and instead just uses the API provided by JSON-B (which is expected).
Using either provided or test for an implementation jar is valid.
You would use provided if your application imported, or used any of the classes from yasson.
You would use test if only your tests imported, or used any of the classes from yasson.

Hope that makes sense.