| Summary: | ConcurrentModificationException in JavaExtensionsManager.loadJavaExtensions | ||
|---|---|---|---|
| Product: | [Modeling] Sirius | Reporter: | Aurelien Pupier <apupier> |
| Component: | Core | Assignee: | Pierre-Charles David <pierre-charles.david> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P5 | CC: | julien.dupont, pierre-charles.david |
| Version: | 4.0.0 | Keywords: | triaged |
| Target Milestone: | 5.1.0 | ||
| Hardware: | PC | ||
| OS: | Windows NT | ||
| See Also: |
https://git.eclipse.org/r/103469 https://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=adaec6b4aab67d60ff93b54bae7ade95aa8fbadc |
||
| Whiteboard: | |||
|
Description
Aurelien Pupier
Hi Aurelien, Without use case reproduction it's almost impossible to correct the bug. Could you give us steps to reproduce problem. Thanks. Regards, As said previously, I don't know the steps to reproduce. I was going through the tutorial given @EclipseCon France and just had time to take the stacktrace. I don't know the exact steps either, but I'm pretty sure I've already been hit by that stack too, and looking at the code it can clearly be made more robust regarding to this, so I'm marking this as triaged. One possible hint for a reproduction scenario (which would be compatible with Aurelien's mention that he was following the EclipseCon tutorial) would be the properties view (installed in the package provided for the tutorial) which launches its own refresh at the same time as the diagram refresh. The properties view refresh code share the same interpreter, and programmatically register its own service class, which would concurently modify the JavaExtensionsManager.imports field, which corresponds to the LinkedHashMap in the stack. Moving out of 4.1.0, as I have not seen it happen in a while, and have no clear reproduction scenario. From reading the code itself, it is clear that it *can* happen, but identifying user-level scenarios which could trigger it is not easy. I'm pushing it to 4.1.1 instead of 5.0.0 to try and have a look if time permits, but with no guarantee. New Gerrit change created: https://git.eclipse.org/r/103469 (In reply to Eclipse Genie from comment #6) > New Gerrit change created: https://git.eclipse.org/r/103469 Analysis: The 'imports' HashSet is private, and we never leak a reference to it to anybody (getImports() returns an immutable copy). The 'for' loop where the CME occurs does not call (directly or not) any of the mutation methods which modify 'imports', so it must come from another thread hitting on such a (public) mutation method while we're iterating. It should be enough to synchonize all public methods which end up accessing to 'imports' (loadJavaExtensions() is already synchronized via reloadIfNeeded() which is its only caller). Gerrit change https://git.eclipse.org/r/103469 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=adaec6b4aab67d60ff93b54bae7ade95aa8fbadc Fixed (probably) by adaec6b4aab67d60ff93b54bae7ade95aa8fbadc. Can not be verified: missing or incomplete reproduction information. Steps to reproduce: we don't have any, the only check we can do is verify that the patch is safe. It's been merged for a while with no regression identified, si I'll mark this as validated myself. Available in Sirius 5.1.0, see https://wiki.eclipse.org/Sirius/5.1.0. |