Bug 148066

Created attachment 70335 [details]
Proposed change

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.

Whoops, there is a forgotten "assert mutex != null;" statement in execute(), I will remove it before committing.

[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.

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.

Created attachment 70432 [details]
Proposed change addressing JG02 and JG03

Looks good.

If no one disagrees, I would like to commit this tomorrow.

This looks like an API change. openide.nodes now "imports" org.netbeans.api.project.ProjectManager. Otherwise OK.

#dabaacb8e0c2

Marking as fixed.

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

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?

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).

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.

Such P3s need to be evaluated for P2 status.

> 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.

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.)

*** Issue 150540 has been marked as a duplicate of this issue. ***

Perhaps it could be displayed when runs in non AWT thread?

Please do not reopen. Track the stack trace in the original issue. This is FIXED as intended.