Bug 190452 - IDE hangs on SuiteUtils.removeModule firing FS events under projectmanager lock
IDE hangs on SuiteUtils.removeModule firing FS events under projectmanager lock
Status: NEW
Product: apisupport
Classification: Unclassified
Component: Project
6.x
All All
: P3 (vote)
: 8.0
Assigned To: Martin Kozeny
issues@apisupport
: RANDOM, THREAD
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-09-20 08:55 UTC by Egor Ushakov
Modified: 2013-08-09 12:03 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
:


Attachments
thread dump (21.10 KB, text/plain)
2010-09-20 08:55 UTC, Egor Ushakov
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Egor Ushakov 2010-09-20 08:55:51 UTC
Created attachment 102078 [details]
thread dump

I changed suite components and IDE hanged, see thread dump
Comment 1 Jesse Glick 2010-09-22 14:04:18 UTC
(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).
Comment 2 Jaroslav Tulach 2010-12-06 12:45:01 UTC
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);
Comment 3 Jesse Glick 2010-12-06 16:40:52 UTC
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.
Comment 4 Jaroslav Tulach 2010-12-07 08:05:10 UTC
(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.
Comment 5 Jesse Glick 2010-12-07 15:24:00 UTC
(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.)
Comment 6 Jesse Glick 2012-06-13 22:03:03 UTC
Not for 7.2.


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