Calling DefaultKafkaHeaderMapper constructor with the same ObjectMapper instance may result memory leak

[sbozcan]

In what version(s) of Spring for Apache Kafka are you seeing this issue?

2.8.2

Describe the bug

DefaultKafkaHeaderMapper is registering a SimpleModule in constructor to the given ObjectMapper instance. Since the SimpleModule's are unnamed modules, IGNORE_DUPLICATE_MODULE_REGISTRATIONS configuration of ObjectMapper doesn't handle multiple registration of this module. This is problematic and can cause a memory leak if the DefaultKafkaHeaderMapper(ObjectMapper mapper) constructor is called with the same ObjectMapper instance multiple times.

To Reproduce

Call the DefaultKafkaHeaderMapper(ObjectMapper mapper) function with same ObjectMapper instance multiple times.

Expected behavior

A new SimpleModule would be registered to the ObjectMapper beacuse of the new SimpleModule().addDeserializer(MimeType.class, new MimeTypeJsonDeserializer()) call.

Solution

This problematic situation can be avoided simply by giving a name to the SimpleModule created as below:

this.objectMapper.registerModule(new SimpleModule("mimeTypeJsonDeserializer", Version.unknownVersion())
						.addDeserializer(MimeType.class, new MimeTypeJsonDeserializer()));

[garyrussell]

Thanks for pointing this out, but it would be rather unusual to create multiple header mappers.

Contributions are welcome.

[artembilan]

Yeah... That's why I always say that it is better to not mutate an externally provided objects:

		this.objectMapper = objectMapper;
		this.objectMapper
				.registerModule(new SimpleModule().addDeserializer(MimeType.class, new MimeTypeJsonDeserializer()));

[garyrussell]

That's a good point; that code should only be applied to an internally created OM.

[garyrussell]

Maybe it can now be removed altogether (at least in 3.1); it was introduced a long time ago for backwards compatibility with old, unsupported, versions.

#892

[artembilan]

I guess so. Since it is an internal API we probably can remove it without any side-effects.