MultiThread + compare() = ATTEMPT_TO_OVERWRITE_EXISTING_JAVERSTYPE_MAPPING

[Adrien-dev25]

Description
I am using Javers in a Multithread context to compare instances of several kinds of object. If two kind of objects have a property of type List<SomeThing> then the javers.compare(a, b) throws from time to time an ATTEMPT_TO_OVERWRITE_EXISTING_JAVERSTYPE_MAPPING exception.

Steps To Reproduce
Here is a short example that very often throws the ATTEMPT_TO_OVERWRITE_EXISTING_JAVERSTYPE_MAPPING exception.
If you decrease the number of threads of the Executors to 1, the exception is never thrown.

public class MyApp {

	private static final Javers JAVERS = JaversBuilder.javers().build();

	@Getter
	@Setter
	@NoArgsConstructor
	static class StorageA {
		private List<String> items;
	}

	@Getter
	@Setter
	@NoArgsConstructor
	static class StorageB {
		private List<String> items;
	}

	public static void main(final String[] args) {
		// If you decrease the number of thread from 10 to 1 the exception diseapers
		ExecutorService executorService = Executors.newFixedThreadPool(10);
		try {
			List<CompletableFuture<Void>> futures = new ArrayList<>();
			for (int i = 0; i < 5; i++) {
				futures.add(CompletableFuture.runAsync(() -> JAVERS.compare(new StorageA(), new StorageA()),
						executorService));
				futures.add(CompletableFuture.runAsync(() -> JAVERS.compare(new StorageB(), new StorageB()),
						executorService));
			}
			CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])).join();
		} finally {
			executorService.shutdown();
		}
	}
}

Javers' Version
7.7.0

Root cause
In my opinion, the bug comes from the class TypeMapperEngine, the synchronize(javaType) is too "weak" : https://github.com/javers/javers/blob/master/javers-core/src/main/java/org/javers/core/metamodel/type/TypeMapperEngine.java#L104

The synchronisation should be done on a dedicated LOCK object instead of the javaType function parameter.

[bartoszwalacik]

Hi Adrien, could be, you mean that these vars on witch we sync might point to different objects, when they point to same type but with different parameters? If so, we should find a way to lock on a raw type.
If you come up with a PR , i will merge.

[Adrien-dev25]

yes, the sync is done on Type java.util.List<java.lang.String> for which there are 2 different instances in my example : one for StorageA.items and another for StorageB.items, but in the ConcurrentHashMap mappedTypes you use as key javaType.toString() which is the same for both instances and generate the collision. The sync should be done exactly on the same object as the key inserted in the Hashmap or on a dedicated lock.

I imagine 2 possible fix :

• Change the type of the mappedTypes : Map<Type, JaversType> instead of Map<String, JaversType>
• Do the sync on a dedicated object : private final static LOCK_FOR_MAPPED_TYPES = new Object()

Do you have a preference ?

[Adrien-dev25]

Hi Bartosz
I wrote a PR with the test case that demonstrates the bug and with a fix using a dedicated LOCK Object for the synchronisation :
#1428

[bartoszwalacik]

Thanks @Adrien-dev25 , will CR it

[bartoszwalacik]

Well done, PR merged

[bartoszwalacik]

released in 7.7.1
thanks @Adrien-dev25 for this valuable contribution

[Adrien-dev25]

Thank for the release, but I think you did a release with a wrong tag 7.1.1 instead of 7.7.1 ?

[Adrien-dev25]

@bartoszwalacik Would it be possible to release the correct tag version : 7.7.1 (instead of 7.1.1) ? 🙏