Bug 148066 - Detect deadlocks between ProjectManager.mutex() and Children.MUTEX
Detect deadlocks between ProjectManager.mutex() and Children.MUTEX
Status: RESOLVED FIXED
Product: platform
Classification: Unclassified
Component: Nodes
6.x
All All
: P2 (vote)
: 6.x
Assigned To: Andrei Badea
issues@platform
: THREAD
Depends on:
Blocks: 150540 161596
  Show dependency treegraph
 
Reported: 2008-09-23 16:03 UTC by Andrei Badea
Modified: 2009-04-03 10:19 UTC (History)
7 users (show)

See Also:
Issue Type: ENHANCEMENT
:


Attachments
Proposed change (7.94 KB, text/plain)
2008-09-23 16:04 UTC, Andrei Badea
Details
Proposed change addressing JG02 and JG03 (9.17 KB, text/plain)
2008-09-24 12:24 UTC, Andrei Badea
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrei Badea 2008-09-23 16:03:45 UTC
A common type of deadlock exposed by the background saving of the project properties is between ProjectManager.mutex()
and Children.MUTEX. Usually the AWT event dispatching thread is holds Children.MUTEX and needs PM.mutex(), whereas the
thread which is saving the project holds PM.mutex() and needs to acquire Children.MUTEX. There is no known reason to
acquire Children.MUTEX under PM.mutex() apart from the default implementation of DataObject.getLookup(), which was fixed
in issue 147492. Therefore I suggest that Children.MUTEX detect that it is being acquired under PM.mutex() and throw an
exception.
Comment 1 Andrei Badea 2008-09-23 16:04:39 UTC
Created attachment 70335 [details]
Proposed change
Comment 2 Andrei Badea 2008-09-23 16:06:49 UTC
Please comment on the attached patch. I would like to integrate it as soon as possible before 6.5 is branched and
assertions turned off.
Comment 3 Andrei Badea 2008-09-23 17:08:57 UTC
Whoops, there is a forgotten "assert mutex != null;" statement in execute(), I will remove it before committing.
Comment 4 Jesse Glick 2008-09-23 18:36:00 UTC
[JG01] Perhaps it would be better to move the unit test to projectapi so you can actually use the real
ProjectManager.mutex(). BTW does ClassLoader even let you return PM.class?


[JG02] Good idea to make the unit test silently return in case assertions are off. (Normally they should be on for all
unit tests, but you never know.)


[JG03] Should probably document this use of reflection in openide.nodes/arch.xml.
Comment 5 Andrei Badea 2008-09-24 12:17:00 UTC
Re JG01: If I can easily fake ProjectManager.mutex() -- as I do -- I would prefer to leave it in openide.nodes, near the
tested code. If I use the real mutex I would have to mess with the ProjectManager.mutex field to allow the test to
garbage-collect it, then set it back to a new instance to allow other tests to pass.

I was surprised that ClassLoader lets me return PM.class from loadClass(), but it does. Class.forName() checks the class
name, but the class loader doesn't seem to. That's why I use CL.loadClass() instead of Class.forName() in PMDeadlockChecker.

Re JG02, JG03: will do.
Comment 6 Andrei Badea 2008-09-24 12:24:36 UTC
Created attachment 70432 [details]
Proposed change addressing JG02 and JG03
Comment 7 Jesse Glick 2008-09-24 16:30:09 UTC
Looks good.
Comment 8 Andrei Badea 2008-09-25 10:28:50 UTC
If no one disagrees, I would like to commit this tomorrow.
Comment 9 Jaroslav Tulach 2008-09-25 13:39:01 UTC
This looks like an API change. openide.nodes now "imports" org.netbeans.api.project.ProjectManager. Otherwise OK.
Comment 10 Andrei Badea 2008-10-01 14:37:08 UTC
#dabaacb8e0c2
Comment 11 Andrei Badea 2008-10-01 14:37:43 UTC
Marking as fixed.
Comment 12 Quality Engineering 2008-10-03 14:39:06 UTC
Integrated into 'main-golden', will be available in build *200810031107* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/dabaacb8e0c2
User: Andrei Badea <abadea@netbeans.org>
Log: #148066: Detect deadlocks between ProjectManager.mutex() and Children.MUTEX
Comment 13 Lukas Hasik 2008-10-03 15:56:34 UTC
nice, it seems that now we can expect looot of ISE from any logical view. #149080, #149084, #149089

Is it necessary to throw new ISE? QE has alredy tested all the IDE. Now there will be ISEs when working with nodes. Do
you thing that we are able to retest everything again before RC1 goes out?

WHY, WHY, we are working on ENHANCEMENTS in bug fixing mode?
Comment 14 Jesse Glick 2008-10-03 16:03:55 UTC
It is an "enhancement" to find otherwise difficult-to-reproduce bugs which are potentially serious (deadlocks).

The ISE is thrown only when assertions are on. For release builds, which RC1 should be considered, the behavior is
unchanged (i.e. the IDE will usually be fine but may on occasion deadlock).
Comment 15 Lukas Hasik 2008-10-03 16:19:14 UTC
ok, it is better but I didn't get why we added it now. The only result will be P3s that will appear in next days but
won't be fixed. IMO, we could add it after cloning 65.
Comment 16 Jesse Glick 2008-10-03 16:42:15 UTC
Such P3s need to be evaluated for P2 status.
Comment 17 Andrei Badea 2008-10-03 17:02:50 UTC
> ok, it is better but I didn't get why we added it now.

In an attempt to find as many potential deadlocks as possible before we release 6.5. Better to fix them than to force
6.5 users to restart because the IDE deadlocked.
Comment 18 mslama 2009-03-16 13:58:21 UTC
See issue #150540 where modal dialog under PM.mutex() is displayed and some nodes are synchronously in AWT thread
repainted ie. Children.MUTEX is requested. It is valid use case and there is no clear way how to split repainting from
displaying dialog.

Steps to reproduce:
1.Open J2SE project
2.Open, modify and save build-impl.xml.
3.Expand project in Projects tab so that Libraries node and it children are displayed.
3.Open project properties, goto Libraries, Run tab add any library and press OK. -> exception is displayed. (If
exception is not displayed restart IDE and try again.)
Comment 19 mslama 2009-03-16 13:58:47 UTC
*** Issue 150540 has been marked as a duplicate of this issue. ***
Comment 20 mslama 2009-03-16 14:06:34 UTC
Perhaps it could be displayed when runs in non AWT thread?
Comment 21 Jesse Glick 2009-03-16 15:38:25 UTC
Please do not reopen. Track the stack trace in the original issue. This is FIXED as intended.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo