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.

Bug 40531 - Deadlock in I18N wizard
Summary: Deadlock in I18N wizard
Status: VERIFIED FIXED
Alias: None
Product: java
Classification: Unclassified
Component: I18N (show other bugs)
Version: 3.x
Hardware: PC Linux
: P2 blocker (vote)
Assignee: Marian Petras
URL:
Keywords: RANDOM, THREAD
Depends on:
Blocks:
 
Reported: 2004-02-26 16:42 UTC by _ pkuzel
Modified: 2004-03-11 16:08 UTC (History)
4 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
The deadlock (6.03 KB, text/plain)
2004-02-26 16:43 UTC, _ pkuzel
Details
sequence diagram of deadlocked threads (15.70 KB, image/png)
2004-03-01 14:14 UTC, Marian Petras
Details
Proposed patch (1.28 KB, patch)
2004-03-08 17:34 UTC, _ pkuzel
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description _ pkuzel 2004-02-26 16:42:30 UTC
While running tha wizard on tasklist/doscan
sources I got deadlock on 3rd page (I guess it was
3rd).

Attached.
Comment 1 _ pkuzel 2004-02-26 16:43:00 UTC
Created attachment 13684 [details]
The deadlock
Comment 2 Marian Petras 2004-03-01 14:12:34 UTC
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.
Comment 3 Marian Petras 2004-03-01 14:14:03 UTC
Created attachment 13745 [details]
sequence diagram of deadlocked threads
Comment 4 Marian Petras 2004-03-01 14:39:09 UTC
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).
Comment 5 Marian Petras 2004-03-04 10:13:47 UTC
It should be fixed in NetBeans 3.6.
Comment 6 _ pkuzel 2004-03-04 14:00:19 UTC
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?
Comment 7 _ pkuzel 2004-03-04 14:06:38 UTC
Whole NbPresenter.propertyChange is inherently wrong as it calls
toSwing/AWT from arbitrary thread. Reassiging...
Comment 8 _ pkuzel 2004-03-04 14:12:44 UTC
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)
Comment 9 David Simonek 2004-03-08 15:36:26 UTC
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)
Comment 10 _ pkuzel 2004-03-08 16:34:53 UTC
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.

Comment 11 _ ttran 2004-03-08 17:07:34 UTC
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
Comment 12 _ pkuzel 2004-03-08 17:16:17 UTC
I filled <http://www.netbeans.org/issues/show_bug.cgi?id=40835> and
will invokeLater from i18n code until resolved in core.
Comment 13 _ ttran 2004-03-08 17:19:31 UTC
thanks very much
Comment 14 _ pkuzel 2004-03-08 17:34:57 UTC
Created attachment 13881 [details]
Proposed patch
Comment 15 _ pkuzel 2004-03-08 17:36:05 UTC
Patch attached. Marian could you review, please?
Comment 16 Marian Petras 2004-03-09 14:09:46 UTC
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.
Comment 17 Marian Petras 2004-03-09 14:12:22 UTC
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'.
Comment 18 Marian Petras 2004-03-09 14:18:50 UTC
It is fixed in the trunk - marking is FIXED.
Comment 19 _ pkuzel 2004-03-09 15:21:20 UTC
The fix is nice and can be applied to release36 branch.
Comment 20 Milan Kubec 2004-03-10 09:40:25 UTC
QE, please, verify that wizard functionality is not broken (on trunk
build). Thanks.
Comment 21 ehucka 2004-03-11 15:01:11 UTC
I don't see any new problem with i18nwizard.
Comment 22 Marian Petras 2004-03-11 16:08:45 UTC
The fix is now integrated also to branch 'release36'.