Bug 277375 - NPE shutting down with compare editor open
Summary: NPE shutting down with compare editor open
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Team (show other bugs)
Version: 3.5   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Platform Team Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: investigate
: 278804 288360 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-05-21 15:01 EDT by John Arthorne CLA
Modified: 2019-09-06 16:15 EDT (History)
9 users (show)

See Also:


Attachments
Fix v01 (1.39 KB, patch)
2009-07-07 06:23 EDT, Tomasz Zarna CLA
no flags Details | Diff
mylyn/context/zip (15.10 KB, application/octet-stream)
2009-07-07 06:23 EDT, Tomasz Zarna CLA
no flags Details
Fix (for NPE but not for its root cause) (1.56 KB, patch)
2009-11-19 11:13 EST, Dani Megert CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Arthorne CLA 2009-05-21 15:01:55 EDT
I20090520-2000

I shutdown this morning with several compare editors open, and got one of these errors in my log for each editor. Note getActivePage() will return null during shutdown.

!ENTRY org.eclipse.team.core 4 2 2009-05-21 09:55:57.671
!MESSAGE Problems occurred when invoking code from plug-in: "org.eclipse.team.core".
!STACK 0
java.lang.NullPointerException
	at org.eclipse.team.ui.synchronize.SaveableCompareEditorInput.closeEditor(SaveableCompareEditorInput.java:292)
	at org.eclipse.team.internal.ui.mapping.ModelCompareEditorInput.access$0(ModelCompareEditorInput.java:1)
	at org.eclipse.team.internal.ui.mapping.ModelCompareEditorInput$1.cacheDisposed(ModelCompareEditorInput.java:55)
	at org.eclipse.team.internal.core.Cache$1.run(Cache.java:96)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
	at org.eclipse.team.internal.core.Cache.dispose(Cache.java:94)
	at org.eclipse.team.core.mapping.provider.SynchronizationContext.dispose(SynchronizationContext.java:75)
	at org.eclipse.team.core.subscribers.SubscriberMergeContext.dispose(SubscriberMergeContext.java:104)
	at org.eclipse.team.ui.synchronize.ModelSynchronizeParticipant.dispose(ModelSynchronizeParticipant.java:252)
	at org.eclipse.team.internal.ui.synchronize.SynchronizeManager.dispose(SynchronizeManager.java:579)
	at org.eclipse.team.internal.ui.TeamUIPlugin.stop(TeamUIPlugin.java:242)
	at org.eclipse.osgi.framework.internal.core.BundleContextImpl$2.run(BundleContextImpl.java:840)
	at java.security.AccessController.doPrivileged(AccessController.java:242)
	at org.eclipse.osgi.framework.internal.core.BundleContextImpl.stop(BundleContextImpl.java:833)
	at org.eclipse.osgi.framework.internal.core.BundleHost.stopWorker(BundleHost.java:474)
	at org.eclipse.osgi.framework.internal.core.AbstractBundle.suspend(AbstractBundle.java:546)
	at org.eclipse.osgi.framework.internal.core.Framework.suspendBundle(Framework.java:1098)
	at org.eclipse.osgi.framework.internal.core.StartLevelManager.decFWSL(StartLevelManager.java:593)
	at org.eclipse.osgi.framework.internal.core.StartLevelManager.doSetStartLevel(StartLevelManager.java:261)
	at org.eclipse.osgi.framework.internal.core.StartLevelManager.shutdown(StartLevelManager.java:216)
	at org.eclipse.osgi.framework.internal.core.InternalSystemBundle.suspend(InternalSystemBundle.java:266)
	at org.eclipse.osgi.framework.internal.core.Framework.shutdown(Framework.java:685)
Comment 1 Tomasz Zarna CLA 2009-06-02 11:41:47 EDT
*** Bug 278804 has been marked as a duplicate of this bug. ***
Comment 2 Olivier Thomann CLA 2009-06-02 13:50:37 EDT
I also got it twice this morning using I20090527-2000.
Comment 3 Tomasz Zarna CLA 2009-06-03 05:46:51 EDT
I don't know how this could happen but it appears that disposing the Synchronize Merger happens before Compare Editor Input removes[2] its context listener[3]. Or what is more probable the editor input doesn't remove the listener (on time / at all) or fails to do so.

Guys, can any of you provide some reproducible steps? Does it happen all the time? You did regular synchronization or were in a middle of a merging process? What model did you use (All models, Java, Workspace...)? And please don't tell me you all have dual-core machines.

[1] org.eclipse.team.internal.ui.synchronize.SynchronizeManager.dispose()
[2] org.eclipse.team.internal.ui.mapping.ModelCompareEditorInput.handleDispose() line 84 in 1.41
[3] org.eclipse.team.internal.ui.mapping.ModelCompareEditorInput.contextListener, it's more like a cache listener btw
Comment 4 John Arthorne CLA 2009-06-03 08:02:45 EDT
This happens every time I shut down with a java compare editor open. I have an incoming or outgoing change in the sync view, click on a Java file to open the editor, shutdown, and the NPE always happens. As I mentioned in comment #0, it seems pretty clear that getActivePage() can return null and you don't check for this. The active page will be null during shutdown.
Comment 5 John Arthorne CLA 2009-06-03 08:03:30 EDT
And, my sync model is set to "All Models".
Comment 6 Tomasz Zarna CLA 2009-06-03 08:51:58 EDT
John, you're definitely right about the getActivePage(), but from my observation (debugging) it looks like the code should not be even run in that specific case. The compare editor input should dispose itself earlier and among other things remove the listener. Thus it would no longer listen for cache disposal -> try to close the editor -> call getActivePage(). 

On the other hand, I must be missing something, because apparently it may go other way round and this bug proves it.
Comment 7 Olivier Thomann CLA 2009-06-09 15:28:44 EDT
Will this be fixed for 3.5.1?
Comment 8 Tomasz Zarna CLA 2009-06-16 05:47:15 EDT
Olivier, so far I wasn't able to reproduce this issue, so one thing I can tell you for sure is that we will investigate the problem further during 3.6. If it turns out to be a serious problem we will consider it for 3.5.1.
Comment 9 John Arthorne CLA 2009-06-16 10:57:47 EDT
I think minimally you need to add the null check on the call to getActivePage(). I see this in my log every day and it looks very bad.
Comment 10 Brian Vosburgh CLA 2009-06-30 14:50:31 EDT
I have a .log with over 700KB of only this stacktrace, spanning only a week's worth of usage. Easily reproducible in *my* workspace. :-)
Comment 11 Tomasz Zarna CLA 2009-07-07 06:23:08 EDT
Created attachment 140943 [details]
Fix v01

A temporary fix suggested by John.
Comment 12 Tomasz Zarna CLA 2009-07-07 06:23:15 EDT
Created attachment 140944 [details]
mylyn/context/zip
Comment 13 Olivier Thomann CLA 2009-07-28 10:03:26 EDT
Will this be fixed soon?
I am still getting entries in my .log file every day.
Comment 14 Szymon Brandys CLA 2009-07-28 10:08:19 EDT
Tomasz, we can release the temporary fix, right?
Comment 15 Tomasz Zarna CLA 2009-07-30 05:23:24 EDT
(In reply to comment #14)
> Tomasz, we can release the temporary fix, right?

Sure, I was about to do it as soon as I get back from my short vacation. I'm sorry for the fact that releasing the temp. fix took so long. It's now available in builds >N20090729-2000. It would be great if any of you guys experiencing the issue could let me know whether it changed anything...

I'm leaving the bug open until we find the real cause of the problem and the proper fix.
Comment 16 Tomasz Zarna CLA 2009-08-17 11:50:33 EDT
Guys, any feedback? Olivier you were the last one pinging me about the issue, has the fix helped you get rid of the entries from your log?
Comment 17 John Arthorne CLA 2009-08-17 12:04:20 EDT
Which I-build contained the tagged fix? I still see this in I20090811-0800:

java.lang.NullPointerException
	at org.eclipse.team.ui.synchronize.SaveableCompareEditorInput.closeEditor(SaveableCompareEditorInput.java:295)
	at org.eclipse.team.internal.ui.mapping.ModelCompareEditorInput.access$0(ModelCompareEditorInput.java:1)
	at org.eclipse.team.internal.ui.mapping.ModelCompareEditorInput$1.cacheDisposed(ModelCompareEditorInput.java:55)
	at org.eclipse.team.internal.core.Cache$1.run(Cache.java:96)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
	at org.eclipse.team.internal.core.Cache.dispose(Cache.java:94)
	at org.eclipse.team.core.mapping.provider.SynchronizationContext.dispose(SynchronizationContext.java:75)
	at org.eclipse.team.core.subscribers.SubscriberMergeContext.dispose(SubscriberMergeContext.java:104)
	at org.eclipse.team.ui.synchronize.ModelSynchronizeParticipant.dispose(ModelSynchronizeParticipant.java:252)
	at org.eclipse.team.internal.ui.synchronize.SynchronizeManager.dispose(SynchronizeManager.java:579)
	at org.eclipse.team.internal.ui.TeamUIPlugin.stop(TeamUIPlugin.java:242)

Comment 18 Tomasz Zarna CLA 2009-08-17 12:42:25 EDT
(In reply to comment #17)
> Which I-build contained the tagged fix?
Starting from I20090803-1300. 

Thanks John. Looking at the stack you pasted, it appears that not only the active page can cause us some trouble here. Apparently the active page was not null (the check is in) and the NPE still pops out of: Display display = page.getWorkbenchWindow().getShell().getDisplay(); (line 295, rev 1.20)

As soon as I get back to the office I'll try to reproduce it on different machines (including our Mac).
Comment 19 Olivier Thomann CLA 2009-08-17 15:11:14 EDT
I didn't get it back since the null check has been added­.
Comment 20 Olivier Thomann CLA 2009-08-19 11:48:54 EDT
Using I20090809-2000:

I just got three stack traces this morning:
java.lang.NullPointerException
at org.eclipse.team.ui.synchronize.SaveableCompareEditorInput.closeEditor(SaveableCompareEditorInput.java:295)
at org.eclipse.team.internal.ui.mapping.ModelCompareEditorInput.access$0(ModelCompareEditorInput.java:1)
at org.eclipse.team.internal.ui.mapping.ModelCompareEditorInput$1.cacheDisposed(ModelCompareEditorInput.java:55)
at org.eclipse.team.internal.core.Cache$1.run(Cache.java:96)
at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
at org.eclipse.team.internal.core.Cache.dispose(Cache.java:94)
at org.eclipse.team.core.mapping.provider.SynchronizationContext.dispose(SynchronizationContext.java:75)
at org.eclipse.team.core.subscribers.SubscriberMergeContext.dispose(SubscriberMergeContext.java:104)
at org.eclipse.team.ui.synchronize.ModelSynchronizeParticipant.dispose(ModelSynchronizeParticipant.java:252)
at org.eclipse.team.internal.ui.synchronize.SynchronizeManager.dispose(SynchronizeManager.java:579)
at org.eclipse.team.internal.ui.TeamUIPlugin.stop(TeamUIPlugin.java:242)
at org.eclipse.osgi.framework.internal.core.BundleContextImpl$2.run(BundleContextImpl.java:840)
at java.security.AccessController.doPrivileged(Native Method)
at org.eclipse.osgi.framework.internal.core.BundleContextImpl.stop(BundleContextImpl.java:833)
at org.eclipse.osgi.framework.internal.core.BundleHost.stopWorker(BundleHost.java:475)
at org.eclipse.osgi.framework.internal.core.AbstractBundle.suspend(AbstractBundle.java:546)
at org.eclipse.osgi.framework.internal.core.Framework.suspendBundle(Framework.java:1098)
at org.eclipse.osgi.framework.internal.core.StartLevelManager.decFWSL(StartLevelManager.java:593)
at org.eclipse.osgi.framework.internal.core.StartLevelManager.doSetStartLevel(StartLevelManager.java:261)
at org.eclipse.osgi.framework.internal.core.StartLevelManager.shutdown(StartLevelManager.java:216)
at org.eclipse.osgi.framework.internal.core.InternalSystemBundle.suspend(InternalSystemBundle.java:266)
at org.eclipse.osgi.framework.internal.core.Framework.shutdown(Framework.java:685)
at org.eclipse.osgi.framework.internal.core.Framework.close(Framework.java:583)
at org.eclipse.core.runtime.adaptor.EclipseStarter.shutdown(EclipseStarter.java:409)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:200)
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:597)
at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:559)
at org.eclipse.equinox.launcher.Main.basicRun(Main.java:514)
at org.eclipse.equinox.launcher.Main.run(Main.java:1311)
at org.eclipse.equinox.launcher.Main.main(Main.java:1287)

So the problem is not really fixed or I end up getting the second NPE now.
Comment 21 Tomasz Zarna CLA 2009-08-27 04:53:00 EDT
You're right guys, Fix v01 was a shot in the dark basing on John's suggestion from comment 4. It's my fault, I didn't check the fix. I spent almost half a day trying to reproduce the issue (on a dual-core Mac), but it happened only once (unfortunally I wasn't in the debug mode at that moment). I was able to see bunch of other issues there[1][2][3], but had no luck with reproducing the one you're interested in. I will try to give it another chance soon, until then I hope I will figure out why you're getting the exception each time when I have a hard time reproducing it.

[1] bug 273951, comment 26 and bug 273951, comment 27
[2] bug 287278
[3] bug 277939, comment 15
Comment 22 Tomasz Zarna CLA 2009-09-14 12:15:32 EDT
*** Bug 288360 has been marked as a duplicate of this bug. ***
Comment 23 Olivier Thomann CLA 2009-10-06 17:51:21 EDT
I got a lot of these errors lately.
It seems to be related to how many compare editors I used in my session.

Looking at the code:
	Display display = page.getWorkbenchWindow().getShell().getDisplay();

If the shell is disposed, getShell() might return null.
So this should be checked, shouldn't it ?
Comment 24 Olivier Thomann CLA 2009-11-18 12:14:36 EST
Any progress on this? I am still getting it almost every day. Additional logging might help to identify the problem.
Comment 25 Dani Megert CLA 2009-11-19 03:26:42 EST
I cannot reproduce this on my Windows XP machine and from looking at the shutdown sequence I'm not sure how it happens i.e. when the Team UI plug-in goes down the workbench should still be present.

John and Olivier, is comment 4 still true i.e. can you reproduce this (with all models enabled)?

Something I discovered but which might be unrelated is that org.eclipse.team.ui.synchronize.SaveableCompareEditorInput.closeEditor(boolean) is not called when I simply close the Java or Text compare editor. This smells like a bug in the workflow.
Comment 26 Szymon Brandys CLA 2009-11-19 04:43:33 EST
Assigning the bug to Pawel. It may happen that Pawel will investigate and address the issue during M4, however it's more likely that it will be targeted by M5.
Comment 27 Dani Megert CLA 2009-11-19 11:13:16 EST
Created attachment 152596 [details]
Fix (for NPE but not for its root cause)

This correctly fixes the wrong code in SaveableCompareEditorInput.closeEditor(boolean) BUT it does not fix the root cause of this problem.

Please commit that patch so that Olivier and all others are relieved from the NPE. You can then still investigate the problem by setting a breakpoints.
Comment 28 John Arthorne CLA 2009-11-19 14:40:50 EST
(In reply to comment #25)
> John and Olivier, is comment 4 still true i.e. can you reproduce this (with all models enabled)?

Yes, I still see this on every shutdown. In fact there are often multiple NPE's even though I usually only have one compare editor open. I'm pasting below the most recent stack from today in case the line numbers have changed.

Olivier and I discussed it, and observed that one thing we both do is open the same file in both a compare editor and a normal Java editor. For example double-click a file in the Sync view, and also drag the same file to the editor area to open a regular editor. It seems like compare editor instances are being "leaked", maybe because there are multiple editors on the same IFile at the same time? I know the "saveable" lifecycle is complicated by having multiple "saveables" on the same input at once so maybe this usage pattern is related.

Stack trace using I20091118-1342:

java.lang.NullPointerException
	at org.eclipse.team.ui.synchronize.SaveableCompareEditorInput.closeEditor(SaveableCompareEditorInput.java:295)
	at org.eclipse.team.internal.ui.mapping.ModelCompareEditorInput.access$0(ModelCompareEditorInput.java:1)
	at org.eclipse.team.internal.ui.mapping.ModelCompareEditorInput$1.cacheDisposed(ModelCompareEditorInput.java:55)
	at org.eclipse.team.internal.core.Cache$1.run(Cache.java:96)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
	at org.eclipse.team.internal.core.Cache.dispose(Cache.java:94)
	at org.eclipse.team.core.mapping.provider.SynchronizationContext.dispose(SynchronizationContext.java:75)
	at org.eclipse.team.core.subscribers.SubscriberMergeContext.dispose(SubscriberMergeContext.java:104)
	at org.eclipse.team.ui.synchronize.ModelSynchronizeParticipant.dispose(ModelSynchronizeParticipant.java:252)
	at org.eclipse.team.internal.ui.synchronize.SynchronizeManager.dispose(SynchronizeManager.java:579)
	at org.eclipse.team.internal.ui.TeamUIPlugin.stop(TeamUIPlugin.java:242)
Comment 29 Pawel Pogorzelski CLA 2009-11-20 10:55:24 EST
(In reply to comment #27)
> Created an attachment (id=152596) [details]
> Fix (for NPE but not for its root cause)

Thanks Dani, the temporary fix is in HEAD.
Comment 30 Dani Megert CLA 2009-11-23 11:37:43 EST
>Olivier and I discussed it, and observed that one thing we both do is open the
>same file in both a compare editor and a normal Java editor. For example
>double-click a file in the Sync view, and also drag the same file to the editor
>area to open a regular editor.
There must be more to it. I am not able to reproduce with those steps.
Comment 31 Pawel Pogorzelski CLA 2010-01-18 13:45:51 EST
I'll take a look at it in M6.
Comment 32 Pawel Pogorzelski CLA 2010-03-09 12:59:11 EST
Moving to 3.6 M7.
Comment 33 Pawel Pogorzelski CLA 2010-04-14 05:54:18 EDT
(In reply to comment #29)
> Thanks Dani, the temporary fix is in HEAD.

John, Olivier, are the error logs gone?
Comment 34 Olivier Thomann CLA 2010-04-14 09:35:15 EDT
I haven't seen it for a long time.
Comment 35 John Arthorne CLA 2010-04-14 10:20:40 EDT
No, I haven't seen it since Dani's workaround was released.
Comment 36 Pawel Pogorzelski CLA 2010-04-19 07:09:43 EDT
Lowering importance and removing the target. I'm not able to reproduce the problem and it doesn't surface any more thanks to Dani's fix. I'll leave the bug open to investigate the problem in the future.
Comment 37 Eclipse Webmaster CLA 2019-09-06 16:15:13 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.