Bug 190452

(Not P1 unless reproducible or common.)

DataNode.fireChange should not be acquiring a fresh lock from a listener callback, I guess. Probably ProjectManagerDeadlockDetector would have rejected the attempt in a dev build (i.e. -ea).

A YAGL in projects is held while generating filesystem change events. CustomizerDialog shall be fixed like:


# This patch file was generated by NetBeans IDE
# It uses platform neutral UTF-8 encoding and \n newlines.
--- Base (BASE)
+++ Locally Modified (Based On LOCAL)
@@ -259,13 +259,19 @@
 
             if ( COMMAND_OK.equals( command ) ) {
                 // Call the OK option listener
+                org.openide.filesystems.FileUtil.runAtomicAction(new Runnable() { 
+                    @Override
+                    public void run() {
                 ProjectManager.mutex().writeAccess(new Mutex.Action<Object>() {
+                            @Override
                     public Object run() {
                         okOptionListener.actionPerformed( e ); // XXX maybe create new event
                         actionPerformed(e, categories);
                         return null;
                     }
                 });
+                    }
+                });
                 
                 final ProgressHandle handle = ProgressHandleFactory.createHandle(NbBundle.getMessage(CustomizerDialog.class, "LBL_Saving_Project_data_progress"));
                 JComponent component = ProgressHandleFactory.createProgressComponent(handle);

Can try suggested patch in apisupport. No idea how to verify fix since I have not heard of this hang happening again. Best would be to also fix DataNode so that this problem cannot happen in the future.

(In reply to comment #3)
> Can try suggested patch in apisupport. No idea how to verify fix since I have

You can write a unit test that deadlocks before the fix and not after the fix.

> not heard of this hang happening again. Best would be to also fix DataNode so
> that this problem cannot happen in the future.

DataNode is an API, apisupport is not. So if there is a way to fix  the behavior in apisupport, it is way less risky than in DataNode.

In general I don't think of EQ.invokeLater and RP.post as a way to fix deadlocks (topic touched few times already). I could do it in DataNode conditionally. Conditionally, if I knew that it is not safe to acquire Mutex. For that I would methods like tryReadAccess, tryWriteAccess. Those methods would have to work on chain of Mutexes (e.g. ensure it is safe to acquire ProjectManager.MUTEX too). I don't plan to introduce these methods for 7.0.

(In reply to comment #4)
> You can write a unit test that deadlocks before the fix and not after the fix.

But the deadlock is not known to be reproducible. Anyway the test will only demonstrate that _something_ about the change causes this exact setup to not deadlock; it does not demonstrate that the deadlock is really being prevented in similar cases.

> DataNode is an API

All node impls are in the presentation layer; modules should not be relying on their exact behavior unless that behavior is specifically documented. But yes it is riskier.

> I don't think of EQ.invokeLater and RP.post as a way to fix deadlocks

It's not pleasant; currently we don't have any better solution in many cases, so this has been the standard fix for nodes which acquire locks at bad times.

> I could do it in DataNode
> conditionally. Conditionally, if I knew that it is not safe to acquire Mutex.
> For that I would methods like tryReadAccess, tryWriteAccess. Those methods
> would have to work on chain of Mutexes (e.g. ensure it is safe to acquire
> ProjectManager.MUTEX too).

Can that work here? DataNode can only know that Children.MUTEX is safe to acquire. (You might expect from Javadoc that postWriteRequest would do what you want, but it doesn't.) In this case the problem is a lock ordering conflict with ProjectManager.mutex, so it is undesirable for DataNode to grab Children.MUTEX in the listener callback even if it is currently uncontended. (Long ago I had a patch for Mutex to allow a priority to be specified in the constructor, so that you could only acquire locks in a predetermined order. Would make this class of problem impossible.)

Not for 7.2.