Community
Participate
Working Groups
After starting up Eclipse with an open review, I got a deadlock. See next comment for relevant traces. I'm pretty certain that this was introduced by https://git.eclipse.org/r/#/c/14571/2/org.eclipse.mylyn.reviews.core/src/org/eclipse/mylyn/reviews/core/spi/remote/emf/RemoteEmfConsumer.java. I guess there was a reason that I hadn't wrapped that call before, but I should have noted it. We need a strategy for adding these adapters in a thread-safe way w/o causing this deadlock. But we should back the initial change out before going any further.
Reverted bad change in: https://git.eclipse.org/r/#/c/14604/
Miles, please post a stack trace next time you encounter a dead lock.
Yep, I meant to. But for the record, the call is from the RemoteEmfConsumer constructor, when we attempt to add the eadapters w/in a modelExec. (That could deadlock in a number of different scenarios.)
Sure, but that's the interesting part here what the context of the dead lock is.
For posterity. :) It's a bit complex. The immediate issue is that we're synchronizing against AbstractRemoteEmfFactory#consumerForLocalKey in two UI waiting threads. In the scenario below, Thread A [UI] is opening the editor and blocking on AbstractRemoteEmfFactory#consumerForLocalKey. But Thread B needs [UI] to complete, and it has already acquired AbstractRemoteEmfFactory#consumerForLocalKey. Thread A: [snip] Thread [main] (Suspended) owns: ArrayList<E> (id=3951) owns: RunnableLock (id=3952) waiting for: HashMap<K,V> (id=3953) GerritReviewRemoteFactory(AbstractRemoteEmfFactory<EParentObjectType,EObjectType,LocalKeyType,RemoteType,RemoteKeyType,ObjectCurrentType>).getConsumerForModel(EParentObjectType, EObjectType) line: 247 PublishUiFactory(AbstractPatchSetUiFactory).getChange() line: 78 PublishUiFactory(AbstractPatchSetUiFactory).isExecutableStateKnown() line: 141 PublishUiFactory(AbstractUiFactory<EObjectType>).createControl(IUiContext, Composite, FormToolkit) line: 75 PatchSetUiFactoryProvider(AbstractUiFactoryProvider<EObjectType>).createControls(IUiContext, Composite, FormToolkit, EObjectType) line: 39 ReviewSetContentSection.createButtons() line: 333 ReviewSetContentSection$2.updated(IRepository, IReview, boolean) line: 139 ReviewSetContentSection$2.updated(EObject, Object, boolean) line: 1 RemoteEmfConsumer$ConsumerAdapter.notifyChanged(Notification) line: 92 Review(BasicNotifierImpl).eNotify(Notification) line: 374 NotificationChainImpl.dispatch(Notification) line: 98 NotificationChainImpl.dispatch() line: 86 RemoteEmfConsumer<EParentObjectType,EObjectType,LocalKeyType,RemoteType,RemoteKeyType,ObjectCurrentType>.applyModel(boolean) line: 252 JobRemoteService$2$1.run() line: 77 RunnableLock.run() line: 35 UISynchronizer(Synchronizer).runAsyncMessages(boolean) line: 135 Display.runAsyncMessages(boolean) line: 3946 Display.observerProc(long, long, long) line: 3536 OS.objc_msgSend(long, long, long, long, long) line: not available [native method] NSMenu.popUpContextMenu(NSMenu, NSEvent, NSView) line: 77 Menu._setVisible(boolean) line: 278 Display.runPopups() line: 4059 Display.readAndDispatch() line: 3614 Workbench.runEventLoop(Window$IExceptionHandler, Display) line: 2701 Workbench.runUI() line: 2665 Workbench.access$4(Workbench) line: 2499 Workbench$7.run() line: 679 Realm.runWithDefault(Realm, Runnable) line: 332 Workbench.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 668 PlatformUI.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 149 IDEApplication.start(IApplicationContext) line: 124 EclipseAppHandle.run(Object) line: 196 EclipseAppLauncher.runApplication(Object) line: 110 EclipseAppLauncher.start(Object) line: 79 EclipseStarter.run(Object) line: 353 EclipseStarter.run(String[], Runnable) line: 180 NativeMethodAccessorImpl.invoke0(Method, Object, Object[]) line: not available [native method] NativeMethodAccessorImpl.invoke(Object, Object[]) line: 57 DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 43 Method.invoke(Object, Object...) line: 601 Main.invokeFramework(String[], URL[]) line: 629 Main.basicRun(String[]) line: 584 Main.run(String[]) line: 1438 Main.main(String[]) line: 1414 Thread B: owns: HashMap<K,V> (id=3953) waiting for: Semaphore (id=3967) Object.wait(long) line: not available [native method] Semaphore.acquire(long) line: 43 UISynchronizer.syncExec(Runnable) line: 168 Display.syncExec(Runnable) line: 4607 RemoteUiService.modelExec(Runnable, boolean) line: 41 RemoteUiService(AbstractRemoteService).modelExec(Runnable) line: 70 RemoteEmfConsumer<EParentObjectType,EObjectType,LocalKeyType,RemoteType,RemoteKeyType,ObjectCurrentType>.<init>(AbstractRemoteEmfFactory<EParentObjectType,EObjectType,LocalKeyType,RemoteType,RemoteKeyType,ObjectCurrentType>, EParentObjectType, EObjectType, LocalKeyType, RemoteType, RemoteKeyType) line: 124 GerritReviewRemoteFactory(AbstractRemoteEmfFactory<EParentObjectType,EObjectType,LocalKeyType,RemoteType,RemoteKeyType,ObjectCurrentType>).getConsumerForLocalKey(EParentObjectType, LocalKeyType) line: 183 GerritTaskDataHandler.updateModelData(TaskRepository, TaskData, ReviewObserver, IProgressMonitor) line: 130 GerritTaskDataHandler.getTaskData(TaskRepository, String, IProgressMonitor) line: 107 GerritConnector.getTaskData(TaskRepository, String, IProgressMonitor) line: 157 SynchronizeTasksJob.synchronizeTask(IProgressMonitor, ITask) line: 245 SynchronizeTasksJob.runInternal(Set<ITask>, IProgressMonitor) line: 218 SynchronizeTasksJob.run(Set<ITask>, IProgressMonitor) line: 153 SynchronizeTasksJob.run(IProgressMonitor) line: 135 SynchronizeRepositoriesJob$1(SynchronizeQueriesJob).run(IProgressMonitor) line: 225 SynchronizeRepositoriesJob.updateQueries(TaskRepository, AbstractRepositoryConnector, Set<RepositoryQuery>, IProgressMonitor) line: 186 SynchronizeRepositoriesJob.run(IProgressMonitor) line: 142 Worker.run() line: 54
(In reply to comment #5) > For posterity. :) It's a bit complex. The immediate issue is that we're > synchronizing against AbstractRemoteEmfFactory#consumerForLocalKey in two UI > waiting threads. In the scenario below, Thread A [UI] is opening the editor and > blocking on AbstractRemoteEmfFactory#consumerForLocalKey. But Thread B needs > [UI] to complete, and it has already acquired Why would we hold that lock? To prevent more than one consumer for the same key?
(In reply to comment #6) > > blocking on AbstractRemoteEmfFactory#consumerForLocalKey. But Thread B needs > > [UI] to complete, and it has already acquired > > Why would we hold that lock? To prevent more than one consumer for the same key? Exactly. There should be 0..1 consumer for any given review/domain_object.
