265317 – [CommonNavigator] LabelProvider disposal handling not correct

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 265317 - [CommonNavigator] LabelProvider disposal handling not correct

Summary: [CommonNavigator] LabelProvider disposal handling not correct

Status:	RESOLVED FIXED

Alias:	None

Product:	Platform
Classification:	Eclipse Project
Component:	UI (show other bugs)
Version:	3.5
Hardware:	Macintosh Mac OS X - Carbon (unsup.)

Importance:	P3 normal (vote)
Target Milestone:	3.5 M6
Assignee:	Francis Upton IV
QA Contact:

URL:
Whiteboard:
Keywords:

Depends on:
Blocks:

Reported:	2009-02-18 11:04 EST by Francis Upton IV
Modified:	2009-03-09 00:37 EDT (History)
CC List:	2 users (show)

See Also:

Attachments
Patch (2.07 KB, patch) 2009-03-01 16:09 EST, Francis Upton IV	no flags	Details \| Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Francis Upton IV

2009-02-18 11:04:28 EST

testHideAvailableExtensions	Success		13.951
testEmptyWindowWorkingSet	Success		0.298
testMissingProjectsInWorkingSet	Success		0.356
testTopLevelWorkingSet	Success		0.248
testTopLevelChange	Success		0.284
testMultipleWorkingSets	Success		0.202
testCategoryWizard	Success		1.028
testNavigatorRootContents	Success		0.274
testNavigatorExtensionEnablement	Error	Graphic is disposed

org.eclipse.swt.SWTException: Graphic is disposed
at org.eclipse.swt.SWT.error(SWT.java:3860)
at org.eclipse.swt.SWT.error(SWT.java:3775)
at org.eclipse.swt.SWT.error(SWT.java:3746)
at org.eclipse.swt.graphics.Image.getBounds(Image.java:607)
at org.eclipse.swt.widgets.TreeItem.calculateWidth(TreeItem.java:230)
at org.eclipse.swt.widgets.Tree.calculateWidth(Tree.java:340)
at org.eclipse.swt.widgets.Tree.setScrollWidth(Tree.java:3322)
at org.eclipse.swt.widgets.Tree.setScrollWidth(Tree.java:3315)
at org.eclipse.swt.widgets.Tree.destroyItem(Tree.java:1023)
at org.eclipse.swt.widgets.TreeItem.destroyWidget(TreeItem.java:331)
at org.eclipse.swt.widgets.Widget.release(Widget.java:1449)
at org.eclipse.swt.widgets.Widget.dispose(Widget.java:670)
at org.eclipse.jface.viewers.AbstractTreeViewer.updateChildren(AbstractTreeViewer.java:2599)
at org.eclipse.jface.viewers.AbstractTreeViewer.internalRefreshStruct(AbstractTreeViewer.java:1862)
at org.eclipse.jface.viewers.TreeViewer.internalRefreshStruct(TreeViewer.java:716)
at org.eclipse.jface.viewers.AbstractTreeViewer.internalRefresh(AbstractTreeViewer.java:1837)
at org.eclipse.jface.viewers.AbstractTreeViewer.internalRefresh(AbstractTreeViewer.java:1793)
at org.eclipse.ui.navigator.CommonViewer.internalRefresh(CommonViewer.java:588)
at org.eclipse.jface.viewers.StructuredViewer$8.run(StructuredViewer.java:1473)
at org.eclipse.jface.viewers.StructuredViewer.preservingSelection(StructuredViewer.java:1381)
at org.eclipse.jface.viewers.TreeViewer.preservingSelection(TreeViewer.java:402)
at org.eclipse.jface.viewers.StructuredViewer.preservingSelection(StructuredViewer.java:1340)
at org.eclipse.jface.viewers.StructuredViewer.refresh(StructuredViewer.java:1471)
at org.eclipse.jface.viewers.ColumnViewer.refresh(ColumnViewer.java:548)
at org.eclipse.ui.navigator.CommonViewer.refresh(CommonViewer.java:378)
at org.eclipse.ui.navigator.CommonViewer.refresh(CommonViewer.java:535)
at org.eclipse.jface.viewers.StructuredViewer.refresh(StructuredViewer.java:1403)
at org.eclipse.ui.tests.navigator.NavigatorTestBase.refreshViewer(NavigatorTestBase.java:162)
at org.eclipse.ui.tests.navigator.OpenTest.testNavigatorExtensionEnablement(OpenTest.java:62)
at org.eclipse.test.EclipseTestRunner.run(EclipseTestRunner.java:354)
at org.eclipse.test.EclipseTestRunner.run(EclipseTestRunner.java:206)
at org.eclipse.test.UITestApplication$3.run(UITestApplication.java:195)
at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35)
at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:133)
at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:3340)
at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3069)
at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2388)
at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2352)
at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2204)
at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:499)
at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:333)
at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:492)
at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:113)
at org.eclipse.test.UITestApplication.runApplication(UITestApplication.java:138)
at org.eclipse.test.UITestApplication.run(UITestApplication.java:60)
at org.eclipse.test.UITestApplication.start(UITestApplication.java:210)
at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:194)
at org.eclipse.equinox.internal.app.MainApplicationLauncher.run(MainApplicationLauncher.java:32)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:368)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179)
at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:556)
at org.eclipse.equinox.launcher.Main.basicRun(Main.java:511)
at org.eclipse.equinox.launcher.Main.run(Main.java:1284)
at org.eclipse.equinox.launcher.Main.main(Main.java:1260)
at org.eclipse.core.launcher.Main.main(Main.java:30)
	0.185
testFindValidExtensions	Success		0.178
testDeactivateTestExtension	Success		0.021
testBindTestExtension	Success		0.025
testTestExtensionVisibility	Success		0.027
testResourceExtensionVisibility	Success		0.018
testVisibleExtensionIds	Success		0.026
testNavigatorRootContents	Success		0.279
testNavigatorExtensionEnablement	Success		0.198
testNavigatorSorterAccess	Success		0.194
testNavigatorRootContents	Success		0.290
testSimpleResFirst	Success		0.187
testBlankLabelProvider	Success		0.212
testSimpleResLast	Success		0.192
testSorterMissing	Error	Graphic is disposed

org.eclipse.swt.SWTException: Graphic is disposed
at org.eclipse.swt.SWT.error(SWT.java:3860)
at org.eclipse.swt.SWT.error(SWT.java:3775)
at org.eclipse.swt.SWT.error(SWT.java:3746)
at org.eclipse.swt.graphics.Image.getBounds(Image.java:607)
at org.eclipse.swt.widgets.TreeItem.calculateWidth(TreeItem.java:230)
at org.eclipse.swt.widgets.Tree.calculateWidth(Tree.java:340)
at org.eclipse.swt.widgets.Tree.setScrollWidth(Tree.java:3322)
at org.eclipse.swt.widgets.Tree.setScrollWidth(Tree.java:3315)
at org.eclipse.swt.widgets.Tree.destroyItem(Tree.java:1023)
at org.eclipse.swt.widgets.TreeItem.destroyWidget(TreeItem.java:331)
at org.eclipse.swt.widgets.Widget.release(Widget.java:1449)
at org.eclipse.swt.widgets.Widget.dispose(Widget.java:670)
at org.eclipse.jface.viewers.AbstractTreeViewer$1.run(AbstractTreeViewer.java:788)
at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70)
at org.eclipse.jface.viewers.AbstractTreeViewer.createChildren(AbstractTreeViewer.java:776)
at org.eclipse.jface.viewers.TreeViewer.createChildren(TreeViewer.java:639)
at org.eclipse.jface.viewers.AbstractTreeViewer.internalExpandToLevel(AbstractTreeViewer.java:1708)
at org.eclipse.jface.viewers.AbstractTreeViewer.internalExpandToLevel(AbstractTreeViewer.java:1718)
at org.eclipse.jface.viewers.AbstractTreeViewer.expandToLevel(AbstractTreeViewer.java:1054)
at org.eclipse.jface.viewers.AbstractTreeViewer.expandToLevel(AbstractTreeViewer.java:1035)
at org.eclipse.jface.viewers.AbstractTreeViewer.expandAll(AbstractTreeViewer.java:1024)
at org.eclipse.ui.tests.navigator.SorterTest.testSorterMissing(SorterTest.java:51)
at org.eclipse.test.EclipseTestRunner.run(EclipseTestRunner.java:354)
at org.eclipse.test.EclipseTestRunner.run(EclipseTestRunner.java:206)
at org.eclipse.test.UITestApplication$3.run(UITestApplication.java:195)
at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35)
at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:133)
at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:3340)
at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3069)
at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2388)
at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2352)
at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2204)
at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:499)
at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:333)
at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:492)
at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:113)
at org.eclipse.test.UITestApplication.runApplication(UITestApplication.java:138)
at org.eclipse.test.UITestApplication.run(UITestApplication.java:60)
at org.eclipse.test.UITestApplication.start(UITestApplication.java:210)
at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:194)
at org.eclipse.equinox.internal.app.MainApplicationLauncher.run(MainApplicationLauncher.java:32)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:368)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179)
at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:556)
at org.eclipse.equinox.launcher.Main.basicRun(Main.java:511)
at org.eclipse.equinox.launcher.Main.run(Main.java:1284)
at org.eclipse.equinox.launcher.Main.main(Main.java:1260)
at org.eclipse.core.launcher.Main.main(Main.java:30)
	0.621
testSorterProperty	Error	Graphic is disposed

org.eclipse.swt.SWTException: Graphic is disposed
at org.eclipse.swt.SWT.error(SWT.java:3860)
at org.eclipse.swt.SWT.error(SWT.java:3775)
at org.eclipse.swt.SWT.error(SWT.java:3746)
at org.eclipse.swt.graphics.Image.getBounds(Image.java:607)
at org.eclipse.swt.widgets.TreeItem.calculateWidth(TreeItem.java:230)
at org.eclipse.swt.widgets.Tree.calculateWidth(Tree.java:340)
at org.eclipse.swt.widgets.Tree.setScrollWidth(Tree.java:3322)
at org.eclipse.swt.widgets.Tree.setScrollWidth(Tree.java:3315)
at org.eclipse.swt.widgets.Tree.destroyItem(Tree.java:1023)
at org.eclipse.swt.widgets.TreeItem.destroyWidget(TreeItem.java:331)
at org.eclipse.swt.widgets.Widget.release(Widget.java:1449)
at org.eclipse.swt.widgets.Widget.dispose(Widget.java:670)
at org.eclipse.jface.viewers.AbstractTreeViewer$1.run(AbstractTreeViewer.java:788)
at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70)
at org.eclipse.jface.viewers.AbstractTreeViewer.createChildren(AbstractTreeViewer.java:776)
at org.eclipse.jface.viewers.TreeViewer.createChildren(TreeViewer.java:639)
at org.eclipse.jface.viewers.AbstractTreeViewer.internalExpandToLevel(AbstractTreeViewer.java:1708)
at org.eclipse.jface.viewers.AbstractTreeViewer.internalExpandToLevel(AbstractTreeViewer.java:1718)
at org.eclipse.jface.viewers.AbstractTreeViewer.expandToLevel(AbstractTreeViewer.java:1054)
at org.eclipse.jface.viewers.AbstractTreeViewer.expandToLevel(AbstractTreeViewer.java:1035)
at org.eclipse.jface.viewers.AbstractTreeViewer.expandAll(AbstractTreeViewer.java:1024)
at org.eclipse.ui.tests.navigator.SorterTest.testSorterProperty(SorterTest.java:72)
at org.eclipse.test.EclipseTestRunner.run(EclipseTestRunner.java:354)
at org.eclipse.test.EclipseTestRunner.run(EclipseTestRunner.java:206)
at org.eclipse.test.UITestApplication$3.run(UITestApplication.java:195)
at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35)
at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:133)
at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:3340)
at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3069)
at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2388)
at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2352)
at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2204)
at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:499)
at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:333)
at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:492)
at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:113)
at org.eclipse.test.UITestApplication.runApplication(UITestApplication.java:138)
at org.eclipse.test.UITestApplication.run(UITestApplication.java:60)
at org.eclipse.test.UITestApplication.start(UITestApplication.java:210)
at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:194)
at org.eclipse.equinox.internal.app.MainApplicationLauncher.run(MainApplicationLauncher.java:32)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:368)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179)
at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:556)
at org.eclipse.equinox.launcher.Main.basicRun(Main.java:511)
at org.eclipse.equinox.launcher.Main.run(Main.java:1284)
at org.eclipse.equinox.launcher.Main.main(Main.java:1260)
at org.eclipse.core.launcher.Main.main(Main.java:30)
	0.205
Properties >>

Back to top

Comment 1 Francis Upton IV

2009-02-18 11:23:09 EST

Boris, I'm guessing this is related to async activity of the decorating label thing.  I'm not sure how this is supposed to work or be synchronized.  These are newly added tests that are failing.  Can you point me in a direction here?

Comment 2 Francis Upton IV

2009-02-19 20:05:34 EST

Boris, do you have any ideas about this?  I can try and reproduce it on my Mac, but I'm wondering if you have seen this sort of thing before.

Comment 3 Boris Bokowski

2009-02-20 11:52:32 EST

(In reply to comment #2)
> Boris, do you have any ideas about this?  I can try and reproduce it on my 
> Mac, but I'm wondering if you have seen this sort of thing before.

I have seen similar problems and documented how I debug them here: http://wiki.eclipse.org/Ninja#Debug_.22Widget_is_disposed.22_exceptions

Let me know if you need help (for example, if you cannot reproduce).

Comment 4 Francis Upton IV

2009-02-24 05:12:45 EST

I have reproduced this and it looks like it's a test problem (these are new tests).  I will have a fix for this before Tuesday's nightly build.

Comment 5 Francis Upton IV

2009-02-24 14:13:45 EST

Boris, I think I need a little advice on this.  Here is the stack trace of the dispose() which is happening when the LabelProvider is going away.  This is what the test case is for, testing that things are orderly when a navigator content extension is disabled.  As you can see from the trace, this causes Jface to dispose the images, but they they still appear in the viewer (the viewer is refreshAll (ed) and expanded).

Do you know how this is suppose to be handled?

java.lang.Exception: Stack trace
	at java.lang.Thread.dumpStack(Thread.java:1082)
	at org.eclipse.swt.graphics.Image.destroy(Image.java:543)
	at org.eclipse.swt.graphics.Resource.dispose(Resource.java:65)
	at org.eclipse.jface.resource.ImageDescriptor.destroyResource(ImageDescriptor.java:176)
	at org.eclipse.jface.resource.DeviceResourceManager.deallocate(DeviceResourceManager.java:63)
	at org.eclipse.jface.resource.AbstractResourceManager.destroy(AbstractResourceManager.java:112)
	at org.eclipse.jface.resource.LocalResourceManager.deallocate(LocalResourceManager.java:91)
	at org.eclipse.jface.resource.AbstractResourceManager.dispose(AbstractResourceManager.java:144)
	at org.eclipse.ui.model.WorkbenchLabelProvider.dispose(WorkbenchLabelProvider.java:118)
	at org.eclipse.ui.internal.navigator.extensions.NavigatorContentExtension$2.run(NavigatorContentExtension.java:246)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37)
	at org.eclipse.ui.internal.navigator.extensions.NavigatorContentExtension.dispose(NavigatorContentExtension.java:232)
	at org.eclipse.ui.internal.navigator.NavigatorContentService.onExtensionActivation(NavigatorContentService.java:806)
	at org.eclipse.ui.internal.navigator.NavigatorActivationService.notifyListeners(NavigatorActivationService.java:254)
	at org.eclipse.ui.internal.navigator.NavigatorActivationService.setActive(NavigatorActivationService.java:187)
	at org.eclipse.ui.internal.navigator.NavigatorActivationService.activateExtensions(NavigatorActivationService.java:344)
	at org.eclipse.ui.tests.navigator.OpenTest.testNavigatorExtensionEnablement(OpenTest.java:60)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:324)
	at junit.framework.TestCase.runTest(TestCase.java:164)
	at junit.framework.TestCase.runBare(TestCase.java:130)
	at junit.framework.TestResult$1.protect(TestResult.java:106)
	at junit.framework.TestResult.runProtected(TestResult.java:124)
	at junit.framework.TestResult.run(TestResult.java:109)
	at junit.framework.TestCase.run(TestCase.java:120)
	at org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:130)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
	at org.eclipse.pde.internal.junit.runtime.RemotePluginTestRunner.main(RemotePluginTestRunner.java:62)
	at org.eclipse.pde.internal.junit.runtime.UITestApplication$1.run(UITestApplication.java:114)
	at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35)
	at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:133)
	at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:3340)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3069)
	at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2388)
	at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2352)
	at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2204)
	at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:499)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:333)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:492)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:113)
	at org.eclipse.pde.internal.junit.runtime.UITestApplication.start(UITestApplication.java:46)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:194)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:368)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:324)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:556)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:511)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1284)
	at org.eclipse.equinox.launcher.Main.main(Main.java:1260)

Comment 6 Francis Upton IV

2009-03-01 16:09:49 EST

Created attachment 127114 [details]
Patch

Comment 7 Francis Upton IV

2009-03-01 16:11:46 EST

Problem was in how the disposal of the label provider interacted with the viewer, this showed up only on the Mac because of SWT calculations there.  Tested on Linux and Mac.

Comment 8 Francis Upton IV

2009-03-01 16:14:31 EST

Released to HEAD N20090301-2000

Comment 9 Boris Bokowski

2009-03-01 16:32:39 EST

I don't think this patch is what you want. It will cause two complete refreshes, where one would suffice if you delay the call to dispose() until after the refresh:

labelProvider.removeListener(
  (ILabelProviderListener) contentService.createCommonLabelProvider());
viewer.refresh();
labelProvider.dispose();

While browsing the code, I found (forgot where) that when this piece of code is executed, a job will be scheduled to refresh the viewer with a bit of a delay. Could the reason for this be that this code may be executed more than once (from a loop), so that you would want to cause only one refresh? If yes, you should not call refresh() eagerly, and move the call to labelProvider.dispose() to that job, until after the refresh has happened.

Comment 10 Francis Upton IV

2009-03-01 23:32:31 EST

Adjusted the fix to just to the refresh() as you suggest.  This is a relatively infrequent operation, so I'm not concerned about hooking it up to the deferred refresh stuff.

Adjusted fix released to HEAD N20090302

Comment 11 Francis Upton IV

2009-03-02 00:21:47 EST

See also bug 228129 which is probably a dup of this.

Comment 12 Francis Upton IV

2009-03-02 08:43:18 EST

Tests passed in 20090301-2000, will close as verified when they pass in 20090302-2000 since the fix was modified.

Comment 13 Francis Upton IV

2009-03-03 10:07:28 EST

N20090302-2000 tests pass.

Comment 14 Francis Upton IV

2009-03-08 09:54:36 EDT

I had further problems with this in testing on Linux and upon reflection (and as Boris pointed out), I'm not happy with this fix.  Changed the refresh from a job to a syncExec().  This will allow the tests to rely on this immediately being refreshed.  Since this refresh happens on the UI thread anyway, I don't see the advantage of a deferred job (except to provide a little extra responsiveness when disabling a content extension, which is a fairly rare operation).  Also, there was not anything done to batch these or anything like that (which would be the other reason to use a deferred job).

Tested this on Linux and the Mac.

Comment 15 Francis Upton IV

2009-03-08 09:55:25 EDT

Released to HEAD N20090308, 35M6

Comment 16 Boris Bokowski

2009-03-08 12:42:46 EDT

(In reply to comment #14)
> Changed the refresh from a
> job to a syncExec().

I am worried about potential deadlocks. Are you sure that the thread you are on holds no locks?

Comment 17 Francis Upton IV

2009-03-08 13:19:08 EDT

(In reply to comment #16)
> (In reply to comment #14)
> > Changed the refresh from a
> > job to a syncExec().
> 
> I am worried about potential deadlocks. Are you sure that the thread you are on
> holds no locks?
> 

I will become sure or fix it.  Thanks.

Comment 18 Francis Upton IV

2009-03-08 16:17:12 EDT

> 
> I will become sure or fix it.  Thanks.
> 
I checked and I can't see where any locks are held when the viewer is refreshed.

Comment 19 Boris Bokowski

2009-03-08 21:38:25 EDT

(In reply to comment #18)
> > 
> > I will become sure or fix it.  Thanks.
> > 
> I checked and I can't see where any locks are held when the viewer is
> refreshed.

I didn't expect any locks being held by the CNF code. The question is, from where is the refresh ultimately triggered? Can you be sure that this always happens on a "pristine" thread, i.e. not from user code that might be in a workspace operation or similar situation where locks are held? Unless you can point me to where the "new Thread()" for this is, I am not convinced.

Have seen too many deadlocks...

Comment 20 Francis Upton IV

2009-03-08 22:20:53 EDT

(In reply to comment #19)
 point me to where the "new Thread()" for this is, I am not convinced.
> 
> Have seen too many deadlocks...
So we could cause a deadlock in the client's code because they are calling us with a lock to for example deactivate a content provider and then the viewer turns around the calls their content provider (under a different of their locks, but they have their locking out of order).  Is that the type of situation you are talking about?  I could not see anything in our (platform UI) code that would do that.

But what are the general rules that we must follow to avoid this sort of situation?  What do we guarantee in our APIs about possibly reentering the user code?  I can understand fixing this case, but I want to understand the broader picture.

Comment 21 Boris Bokowski

2009-03-08 22:50:56 EDT

(In reply to comment #20)
> So we could cause a deadlock in the client's code because they are calling us
> with a lock to for example deactivate a content provider and then the viewer
> turns around the calls their content provider (under a different of their
> locks, but they have their locking out of order).  Is that the type of
> situation you are talking about?  I could not see anything in our (platform UI)
> code that would do that.

Yes, one classical deadlock situation is that some code is executing on the UI thread, trying to acquire a lock that is currently held by a non-UI thread that then does a syncExec without first releasing its lock.

> But what are the general rules that we must follow to avoid this sort of
> situation?

One easy rule is to never use syncExec unless you know that you created the thread yourself and it cannot, at the time of the syncExec, hold any locks.

You could change the syncExec to an asyncExec (the labelProvider.dispose() would have to be moved into the asyncExec then, not sure if it is in the syncExec runnable now). In the tests, you would then have to flush the event queue before you check for results of the refresh operation. See e.g. ViewerTestCase.processEvents:

public void processEvents() {
    Shell shell = fShell;
    if (shell != null && !shell.isDisposed()) {
        Display display = shell.getDisplay();
        if (display != null) {
            while (display.readAndDispatch()) {
            	// loop until there are no more events to dispatch
            }
        }
    }
}

Comment 22 Francis Upton IV

2009-03-09 00:37:09 EDT

> You could change the syncExec to an asyncExec (the labelProvider.dispose()
> would have to be moved into the asyncExec then, not sure if it is in the
> syncExec runnable now). In the tests, you would then have to flush the event
>
Ha!  The labelProvider.dispose() was simply in the calling thread, so this must have always been the UI thread (it did not occur to me that you must always call labelProvider.dispose() on the UI thread).  So in this case, the sync exec is pointless.  But on the other hand, I don't think it's valid for this code to assume that it will be called on the UI thread; if that assumption is removed, then this whole apparatus needs to be moved to an asyncExec or job.