`JsonMapper` not thread-safe when using custom serializers

[stlutz]

Search before asking

• I searched in the issues and found nothing similar.

Describe the bug

When a JsonMapper is configured to serialize an object type differently in two different contexts using @JsonSerialize, e.g. a java.util.Locale should be serialized differently for property "propA" than for "property "propB", then the resulting JsonMapper will sometimes serialize "propB" like it should "propA", if there are multiple threads serializing in parallel using the same JsonMapper.

Version Information

3.1.0

Reproduction

In our serialized JSON, Locales can be represented in two different ways:

1. Usually as an object (e.g. { "code": "en" } for Locale.ENGLISH)
2. In one case as the locale code string itself (e.g. "en" for Locale.ENGLISH)

To achieve this with Jackson we define a custom de-/serializer for all Locales (for case 1) and specifically annotate the one place where it should be different (for case 2).

The test below starts multiple threads in parallel which all use the same JsonMapper to serialize a predefined object structure to JSON. The resulting JSON is then compared against the expected result. However, some of the test runs / threads will produce the wrong output (usually about 10 of the 50 runs on my machine) with the following issue:

Expected	Actual
"en"	{ "code" : "en" }

Note: If every thread creates its own JsonMapper, the test runs successfully.

import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.CyclicBarrier;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.RepeatedTest;

import tools.jackson.databind.SerializationFeature;
import tools.jackson.databind.annotation.JsonDeserialize;
import tools.jackson.databind.annotation.JsonSerialize;
import tools.jackson.databind.json.JsonMapper;
import tools.jackson.databind.util.StdConverter;

class TestReducedExample {

	static class LocaleToStringConverter extends StdConverter<Locale, String> {
		@Override public String convert(Locale value) {
			return value.toString();
		}
	}

	static class StringToLocaleConverter extends StdConverter<String, Locale> {
		@Override public Locale convert(String value) {
			return Locale.of(value);
		}
	}

	static class LocaleToJsonConverter extends StdConverter<Locale, Map<String, String>> {
		@Override public Map<String, String> convert(Locale value) {
			return Map.of("code", value.toString());
		}
	}

	static class JsonToLocaleConverter extends StdConverter<Map<String, String>, Locale> {
		@Override public Locale convert(Map<String, String> localeObj) {
			return Locale.of(localeObj.get("code"));
		}
	}

	@JsonDeserialize(converter = JsonToLocaleConverter.class)
	@JsonSerialize(converter = LocaleToJsonConverter.class)
	interface LocaleMixin {
	}

	static class LocalizedText {
		private Locale locale;
		private String text;

		public LocalizedText() {
		}

		public Locale getLocale() {
			return this.locale;
		}

		public String getText() {
			return this.text;
		}

		public void setLocale(Locale locale) {
			this.locale = locale;
		}

		public void setText(String text) {
			this.text = text;
		}
	}

	interface LocalizedTextMixin {
		@JsonSerialize(converter = LocaleToStringConverter.class)
		@JsonDeserialize(converter = StringToLocaleConverter.class)
		Locale getLocale();
	}

	record MyObject(
			List<Locale> locales,
			List<LocalizedText> localizedTexts
	) {
	}

	private static JsonMapper createMapper() {
		return JsonMapper.builder()
				.enable(SerializationFeature.INDENT_OUTPUT)
				.addMixIn(LocalizedText.class, LocalizedTextMixin.class)
				.addMixIn(Locale.class, LocaleMixin.class)
				.build();
	}

	@RepeatedTest(50)
	void testConcurrentSerialization() throws Throwable {
		String json = """
				{
				  "locales" : [ {
				    "code" : "en"
				  }, {
				    "code" : "de"
				  } ],
				  "localizedTexts" : [ {
				    "locale" : "en",
				    "text" : "text 1"
				  }, {
				    "locale" : "de",
				    "text" : "text 2"
				  } ]
				}""";
		var mapper = createMapper();
		MyObject myObject = mapper.readValue(json, MyObject.class);

		int threadCount = 10;
		CyclicBarrier barrier = new CyclicBarrier(threadCount);
		CopyOnWriteArrayList<Throwable> errors = new CopyOnWriteArrayList<>();
		List<Thread> threads = new java.util.ArrayList<>();

		for (int i = 0; i < threadCount; i++) {
			Thread t = new Thread(() -> {
				try {
					barrier.await();
					for (int j = 0; j < 100; j++) {
						String serializedJson = mapper.writeValueAsString(myObject);
						try {
							Assertions.assertEquals(json.replaceAll("\r\n", "\n"), serializedJson.replaceAll("\r\n", "\n"));
						} catch (Throwable e) {
							errors.add(e);
							return;
						}
					}
				} catch (Throwable e) {
					errors.add(e);
				}
			});
			threads.add(t);
			t.start();
		}

		for (Thread t : threads) {
			t.join();
		}

		if (!errors.isEmpty()) {
			System.out.printf("test failed with %d error(s):%n", errors.size());
			throw errors.getFirst();
		}
	}
}

Expected behavior

JsonMapper should be thread-safe. The test posted in the reproduction should work.

Additional context

No response

[pjfanning]

Copilot created #5822

[pjfanning]

#5826 makes this look like a 3.x regression.

[cowtowncoder]

Fixed in 3.1 for 3.1.1

[stlutz]

Thanks for the quick fix. Do you have a rough idea when 3.1.1 will be released?