Community
Participate
Working Groups
HEAD (including fix for bug 107082) In the History view, show local history for a file, select an old version, and choose 'Get Contents' from the context menu. => NPE below. => Expected: confirmation dialog The problem is that org.eclipse.team.internal.ui.history.LocalHistoryPage.confirmOverwrite() creates an IconAndMessageDialog in a non-UI thread, which worked fine before but is now blocked (with the fix for bug 107082). !ENTRY org.eclipse.team.ui 4 0 2009-07-28 15:03:24.700 !MESSAGE Internal error occurred. !STACK 0 org.eclipse.core.runtime.AssertionFailedException: null argument:Invalid access from non-UI thread. at org.eclipse.core.runtime.Assert.isNotNull(Assert.java:85) at org.eclipse.jface.dialogs.IconAndMessageDialog.getSWTImage(IconAndMessageDialog.java:271) at org.eclipse.jface.dialogs.IconAndMessageDialog.getQuestionImage(IconAndMessageDialog.java:260) at org.eclipse.jface.dialogs.MessageDialog.<init>(MessageDialog.java:189) at org.eclipse.team.internal.ui.history.LocalHistoryPage.confirmOverwrite(LocalHistoryPage.java:641) at org.eclipse.team.internal.ui.history.LocalHistoryPage.access$6(LocalHistoryPage.java:636) at org.eclipse.team.internal.ui.history.LocalHistoryPage$5.run(LocalHistoryPage.java:360) at org.eclipse.team.internal.ui.history.LocalHistoryPage$12.run(LocalHistoryPage.java:605) at org.eclipse.jface.operation.ModalContext$ModalContextThread.run(ModalContext.java:121)
Briefly looking at the code I think we could run whole LocalHistoryPage#confirmOverwrite in the UI thread. Krzysztof if you are still keen on the issue, assign the bug to yourself. Thanks!
NPE should not happen anymore as bug 107082 has been fixed in non-breaking way. However the code in LocalHistoryPage.confirmOverwrite could be improved (MessageDialog should be created in UI thread).
It doesn't make too much sense to mark a bug as 'helpwanted' if it is already assigned to a person ;-)
Helpwanted removed cause Helpreceived ;-) Thanks Krzysiek for looking at this.
Created attachment 143251 [details] Patch Problem is caused by the fact, that in line 602 of LocalHistoryPage method run is given the boolean fork parameter as true, so it is executed in a separate thread. Moving only the creation of new MessageDialog into UI thread inside LocalHistoryPage#confirmOverwrite did not solve the problem if user closes window before the operation is finished (parentSite.getShell() will return null). Changing boolean fork to false ensures that LocalHistoryPage#confirmOverwrite runs whole in the UI thread. This change will not affect any other cases - LocalHistoryPage#getContextMenuAction is private and is called only once for LocalHistoryPage_GetContents message. Final variables, array of integers and running UI thread in LocalHistoryPage#confirmOverwrite are no more needed.
(In reply to comment #5) Have you verified that the operation is still cancelable and shows progress when you run it in the UI thread (e.g. with a slow or broken network connection)?
Created attachment 143381 [details] Patch_2.0 Better patch. Now LocalHistoryPage#confirmOverwrite runs whole in UI thread because it is called before fork in getProgressService() method. Progress bar is shown only after confirmation of file overwiriting, when process really starts. I removed array of integers, final variables and running UI thread from LocalHistoryPage#confirmOverwrite because they are no longer needed.
I believe it works now with the fix for bug 107082. Krzysztof, please look at it, thanks.
It is fixed along with bug 107082.
this bug is connected to bug 107082, and was fixed by it. Marking this as verified also.
