This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Summary: | Deadlock in I18N wizard | ||
---|---|---|---|
Product: | java | Reporter: | _ pkuzel <pkuzel> |
Component: | I18N | Assignee: | Marian Petras <mpetras> |
Status: | VERIFIED FIXED | ||
Severity: | blocker | CC: | dsimonek, jrechtacek, mpetras, ttran |
Priority: | P2 | Keywords: | RANDOM, THREAD |
Version: | 3.x | ||
Hardware: | PC | ||
OS: | Linux | ||
Issue Type: | DEFECT | Exception Reporter: | |
Attachments: |
The deadlock
sequence diagram of deadlocked threads Proposed patch |
Description
_ pkuzel
2004-02-26 16:42:30 UTC
Created attachment 13684 [details]
The deadlock
Confirmed. The problem is that tasks which manipulate with UI elements, are sent to a non-AWT-thread in method I18nWizardDescriptor.Listener.actionPerformed(...) This is a bad scenario - all tasks manipulating with UI should be done in the AWT-event-queue thread. However, the immediate cause of the deadlock is not the described bad scenario - it is the bad threading in org.openide.WizardDescriptor that caused the deadlock. See the attached sequence diagram for details. Created attachment 13745 [details]
sequence diagram of deadlocked threads
The immediate cause could be eliminated by a simple change in class WizardDescriptor. The problematic part of code of WizardDescriptor.java was integrated in revision 1.57. The aim of the change was to post UI handling actions to an AWT-event-queue thread. It worked, but it was sent there asynchronously. The easy fix would be to make the posting synchronous, i.e. use Mutex.EVENT.writeAccess(Action) instead of Mutex.EVENT.writeAccess(Runnable). It is a reasonable fix because it would make WizardDescriptor more robust if run from a non-AWT-event-queue. This goal is apparent from the comment on lines 1569-1571 of WizardDescriptor.java (revision 1.57). It should be fixed in NetBeans 3.6. I evaluate differently. I'd send component validate() requested at org.netbeans.core.windows.services.NbPresenter.propertyChange(NbPresenter.java:817) to AWT using invokeLater. What do you think Jirka? Whole NbPresenter.propertyChange is inherently wrong as it calls toSwing/AWT from arbitrary thread. Reassiging... Or it can be solved at WizardDescriptor (but different lines than Marian pointed out): at org.openide.WizardDescriptor.updateState(WizardDescriptor.java:561) - locked <0x466bcde0> (a org.netbeans.modules.i18n.wizard.I18nWizardDescriptor) at org.netbeans.modules.i18n.wizard.I18nWizardDescriptor.updateState(I18nWizardDescriptor.java:110) Eval: IMO whole WizardDescriptor.updateState is not thread safe, I believe it is designed to be called only from AWT thread. You can find several places where live AWT hierarchy is touched. My suggestions for solving: 1) Marian should assure in I18N wizard that updateState (and possibly also handleNextButton) are called only from AWT thread. 2) Core should improve documentation for WizardDescriptor.updateState, adding comment that it can be invoked only from AWT, possibly also add some kind of warning. Passing to Marian for point 1). (if others agree) Long term I agree. But there is <http://www.netbeans.org/download/release351/javadoc/OpenAPIs/org/openide/doc-files/threading.html#breakdown>. and I think it's DialogDisplayer API. It's announced to be thread safe. Also the synchronized keyword shows different intention. it's not time to argue about which module's fault it is. We need to solve this bug and Dafe suggest the way to fix it in i18n module. We may even call it a workaround for a bug in openide. But that's all what we need to do now. Can someone from i18n module please evaluate Dafe's suggestion? Thanks I filled <http://www.netbeans.org/issues/show_bug.cgi?id=40835> and will invokeLater from i18n code until resolved in core. thanks very much Created attachment 13881 [details]
Proposed patch
Patch attached. Marian could you review, please? Fixed in the trunk. The applied patch is very similar to the one posted by Petr Kuzel - see http://www.netbeans.org/source/browse/i18n/src/org/netbeans/modules/i18n/wizard/I18nWizardDescriptor.java.diff?r1=1.9&r2=1.10&f=u The patch changes the code such that a routine which was in collision with the AWT-event-queue thread is now performed in AWT-event-queue thread, too. Since both originally colliding routines are now performed in the same thread, the deadlock cannot occur any more. The patch should be applied also to NetBeans 3.6 because deadlock can cause data-loss and the patch is pretty simple. Please review the patch so that it may be integrated to branch 'release36'. It is fixed in the trunk - marking is FIXED. The fix is nice and can be applied to release36 branch. QE, please, verify that wizard functionality is not broken (on trunk build). Thanks. I don't see any new problem with i18nwizard. The fix is now integrated also to branch 'release36'. |