From 3f0e34177c7324a64aef603d630ebaeb97ebfa7a Mon Sep 17 00:00:00 2001 From: Marc-Andre Laperle Date: Fri, 10 Feb 2017 12:55:11 -0500 Subject: [PATCH] Bug 511677: Mitigate deadlock closing project during indexer job A deadlock can occur if a project is closing and an indexing job is running. There is a patch on Gerrit that reproduces the lock all the time: https://git.eclipse.org/r/#/c/90603 Thread #1, closes the project (*workspace lock*), removes the binary parser for this project. Around the same time... Thread #2, is just about to start indexing the project (PDOMRebuildTask). It visits the C model to collect source files. Because the binary parser was removed, it reloads the project description, which triggers a listener (CExternalSettingsManager) to get a *workspace lock*. Thread #1 then blocks on cancelling the indexer, which will never happen because it is blocked on getting the workspace lock. This deadlock could happen for any code being called by the indexer thread that gets a workspace lock before the indexer cancels itself. Because so much code can be hooked up to the C model (extensions, listeners, etc), it is difficult to guarantee that this will not happen. - Possible option 1. Changing CExternalSettingsManager to execute asynchronously (Job) breaks other code that expect the operation to be synchronous. Doing this also does not guarantee that other code will not get a workspace lock. - Possible option 2. Cancelling the indexer before the binary parser is removed from the map: does not break any tests but only cover this particular case of the binary parser triggering the project description reload. Any other code that triggers a project description reload (and workspace lock) will still be an issue. Option 2, although incomplete, seems the safest and more reasonable at this point. Judging by the comments in bug 327126, to fix this properly would require quite a big effort that I'm not sure anyone would do. Change-Id: Ida7b45558e4430bc5cc9a1eb5ef25e8d19487c31 Signed-off-by: Marc-Andre Laperle --- .../eclipse/cdt/internal/core/model/CModelManager.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/CModelManager.java b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/CModelManager.java index 8f9938389c6..16890f138b2 100644 --- a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/CModelManager.java +++ b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/CModelManager.java @@ -1400,21 +1400,21 @@ public class CModelManager implements IResourceChangeListener, IContentTypeChang } private void preDeleteProject(IProject project) { + // Stop indexing jobs for this project + CCoreInternals.getPDOMManager().preDeleteProject(create(project)); // Remove binary parsers binaryParsersMap.remove(project); // Stop the binary runner for this project removeBinaryRunner(project); - // Stop indexing jobs for this project - CCoreInternals.getPDOMManager().preDeleteProject(create(project)); } private void preCloseProject(IProject project) { + // Stop indexing jobs for this project + CCoreInternals.getPDOMManager().preCloseProject(create(project)); // Remove binary parsers binaryParsersMap.remove(project); // Stop the binary runner for this project removeBinaryRunner(project); - // Stop indexing jobs for this project - CCoreInternals.getPDOMManager().preCloseProject(create(project)); } public IWorkingCopy[] getSharedWorkingCopies(IBufferFactory factory) {