1.7.0: concurrent modification exception

[JohannesBeranek]

Play version 1.7.0, DEV mode

Looks to me like the amount of concurrentModificationException encountered in DEV mode suddenly skyrocketed. We only had those very very sparsely before, but with play version 1.7.0 they happen about every 10th request.

Always in ApplicationClasses.getApplicationClass. Maybe classes Map should be replaced with a ConcurrentHashMap / add synchronization ?

[aleksandy]

Could you provide stacktrace of the exception?

I assume the cause of ConcurrentModificationException is computeIfAbsent() that has added in 8fad39b. But in my opinion there is no reasons to change map type to concurrent version because in production you use precompiled classes (don't you?) and hashmap will be filled by the all classes before the first concurent request.

Also thread-unsafe map is good choice for readonly work in multithread env because it has more performance because it doesn't care about sync.

[asolntsev]

@aleksandy Yes, we experienced ConcurrentModificationException in DEV mode, but it was also a problem: the application suddenly restarted, sometimes even multiple times.

[aleksandy]

@asolntsev, looks like we don't have a choice. The map should be changed to ConcurrentHashMap. I've send PR.

[JohannesBeranek]

We haven't tried this version in production, as there are a few show stoppers for us to upgrade (this one and the issue with LoggerInit::access()). Still, it's an issue with dev workflow, as this basically makes live-reload unusable in DEV mode.

Of course, you could also just synchronize on write access instead and use a regular HashMap, but for us this isn't really a performance critical part of the application (what are we talking, a few ns maybe?).

[aleksandy]

@JohannesBeranek, could you check if the merged fix solve your issue. And close the issue if so.

[xael-fry]

merges #1408

[xael-fry]

@JohannesBeranek what is the issue about LoggerInit::acces, did you create an issue for it, I didn't find it

Thanks

[JohannesBeranek]

@xael-fry I'll gladly try the changes once I get around to, but that might not be for some time.
As for the issue about LoggerInit::access see issue #1404