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.
Summary: | Lock command completion dialog can conflict with classpath scanner dialog | ||
---|---|---|---|
Product: | obsolete | Reporter: | Jesse Glick <jglick> |
Component: | vcscore | Assignee: | Martin Entlicher <mentlicher> |
Status: | RESOLVED FIXED | ||
Severity: | blocker | CC: | arseniy, dsimonek, issues, mentlicher, mmatula |
Priority: | P2 | Keywords: | RELNOTE, THREAD |
Version: | 4.x | ||
Hardware: | All | ||
OS: | All | ||
Issue Type: | DEFECT | Exception Reporter: | |
Bug Depends on: | |||
Bug Blocks: | 57480 | ||
Attachments: |
Deadlock
Screenshot Thread dump that leads to scanning classpath dialog. The temporary "deadlock" which occurs after the first file is processed. The hack to EventQueue to filter the deadlocking events. A slightly modified partial solution through modification of EventQueue. The working fix to this deadlock. |
Description
Jesse Glick
2005-04-13 22:49:06 UTC
Created attachment 21626 [details]
Deadlock
Created attachment 21627 [details]
Screenshot
Presumably VcsFileSystem.waitForCommand should not be waiting for a modal dialog to be closed unless it can verify that there is no other modal dialog open that would block it? The classpath scanning dialog is opened after VcsFileSystem.waitForCommand() opens the modal dialog. Therefore the classpath scanning dialog can check whether there is not some modal dialog already opened (if it's possible?), and wait after it's closed. Otherwise, every modal dialog in the IDE would have to check whether there was not by chance some other modal dialog opened in between. This particular case can be solved by auto-closing the VCS modal dialog, but I do not think I can find out that another modal dialog was opened over me. Looking into ProjectProperties.PP it looks that the scanning was triggered by (external) modification of the file on which EDIT command was executed... Actually Java is saving into files and refreshing codebase concurrently, as apparent from the attached dump. Created attachment 21648 [details]
Thread dump that leads to scanning classpath dialog.
That way Java will lock into itself until the save is not finished. IMHO it would be better to sync that in Java. But as I looked into J2SEProjectProperties, etc. it seems to be almost impossible to sync it there :-( What is the purpose of the dialog? If it is just a progressbar, could it be closed automatically once the action completes (no matter whether there is another modal dialog or not)? Or could it be non-modal, so that once the operation completes it will not block further processing? It seems that it's not possible to fix this in VCS. Even when I close my modal dialog, the AWT thread keeps waiting for the upper modal dialog. There is no way how to get rid of java.awt.Dialog.show() once there is another dialog upon it. This needs to be fixed in Java. I don't see how we can help here. I guess this is more a project issue rather than java issue (in case it cannot be fixed in VCS). Could the project files be saved outside of the project mutex? Or, Martin, could your dialog be non-modal? Get rid of the dialog, or at least make it nonmodal. Show some message in the status line instead. Later, use Progress API in 4.2 or whenever it becomes available. :-) The dialog *must* be there, because otherwise one would not be able to cancel the command if something goes wrong (so we will have the deadlock anyway if the command runs forever). Running a command in AWT and waiting for it (in AWT) is not not nice by itself and looks like a bug somewhere... This would lead to deadlock immediately in older releases. So the only thing that seems to work is to make the dialog non-modal (I've missed that comment from mmatula, because Issuezilla do not warn you any more when the issue is out-of-date!) So testing the change of modality... "Running a command in AWT and waiting for it (in AWT) is not not nice by itself and looks like a bug somewhere..." - it is vcscore which runs a command and waits for it indefinitely. Clients of the Filesystems API use the EQ routinely. Sorry, but it's not possible. When I make the dialog non-modal, this particular case works, but the dialog as such is not usable. When I e.g. add a field to a read-only Java file, when the dialog is non-modal the IDE looks dead until the command finish. The dialog can not even pait itself. It *must* be modal so that AWT can handle events. I do not thing this is fixable on VCS side, sorry. > it is vcscore which runs a command and waits for it indefinitely. Clients of the
> Filesystems API use the EQ routinely.
Yes, but when VFS.lock() or UQE.confirm() method is called in AWT, VCS can not
do anything about it. It *must* somehow call the command synchronously and wait
for it, because after lock() or confirm() finishes, the file must be already
writable.
Then I would recommend requesting a waiver for 4.1, on the grounds that there are two workarounds available and no fix is forthcoming, and maybe removing support for pessimistic locks completely in 4.2. Dafe, any insights on how different modules should manage clashing modal dialogs? Eeee, no idea for now... I even tried to hack into the EventQueue and postpone the display of the second modal dialog. After I've figured out which events should be ignored and which should be passed on, this seems to work. At least for the first file, then it temporarily deadlocks, because it creates two AWT threads. But the deadlock is only temporary (if the command finishes) so it can be considered as a partial solution. :-) Created attachment 21659 [details]
The temporary "deadlock" which occurs after the first file is processed.
I still don't see why you keep assigning this to java. This needs to be fixed either in VCS (to get rid of the dialog) or in projects - to not do this expensive action in EQ, or under the projects mutex. Reassigning back to VCS. Created attachment 21660 [details]
The hack to EventQueue to filter the deadlocking events.
It seems that all I can do it the attached modification to EventQueue. This postpones the second classpath scanning dialog after mine is closed. But it creates the attached temporary deadlock, that should be fixed in.... (projects? or Java?) We need to have the second "AWT-EventQueue-1" freed up so that processing can continue in the original "AWT-EventQueue-1" ;-) I will try to figure out why org.netbeans.modules.project.ui.ProjectsRootNode$1.run(ProjectsRootNode.java:294) got into the second AWT event queue... The only problem I see in javacore is that it pops up a modal dialog asynchronously (not in direct response to a user action), which is never a good idea, but we already know that is not about to be fixed for 4.1. I don't understand the last patch at all, but if it works, OK... Would it help for ant/project's UserQuestionHandler to wrap the retry block (after the dialog is confirmed) in RequestProcessor? Easy fix if that is all it takes. Not sure what other modules might still suffer from a similar bug, though. O.K. so finally I've came to conclusion that it's really not possible to fully
fix this on VCS side.
I can put there the attached patch of EventQueue, but it might cause unexpected
behavior. The problem is that it creates a new EventDispatchThread and stops the
original one. (This was sort of unexpected to me, I thought that the original
will resume after the new one will finish.) Therefore there is created a new
dispatching thread, the old is left to die.
This makes clear why we can not solve the "temporary" deadlock attached here.
The original AWT event queue is not the dispatching thread any more, but holds
the write Mutex.
> Would it help for ant/project's UserQuestionHandler to wrap the retry block
> (after the dialog is confirmed) in RequestProcessor?
Not really sure what do you mean, but it would not solve the problem described
here - "Configure the CVS working dir to call EDIT locks but *not* prompt."
In this case no UserQuestionException is thrown.
The configuration *with* UQE worked fine for me (without deadlocks).
Created attachment 21661 [details]
A slightly modified partial solution through modification of EventQueue.
So moving to projects to evaluate and possibly resolve the remaining deadlock. The behavior seems to be better, because the IDE resumes after some time. But it looks frozen till the rest of files are not processed. FYI: In PVCS the EDIT command can request an input from the user (e.g. to confirm that a second lock will be applied). In this case we *need* to have a free dispatching thread. Otherwise the command will never finish. I don't see anything to be done about the remaining temporary deadlock. Put this fix in if you want, but it looks dangerous to me; I would recommend just waiving the bug. Please, ask for waiver if it's not going to be fixed in release41. Thanks. The fix by modification of the EventQueue is really too dangerous. I've found that it breaks other things that works now (e.g. when someone add a method to read-only Java file through context menu). Therefore I will not put that patch into trunk. I do not see a way how this can be fixed in VCS, if we should keep the contract that after lock() call the file is writable, we have no option here. :-) Even though I made *reload*, it moved the issue back to projects. It will have to be discussed how to solve this in the future (see issue #57839). This was sort of challenge for me, to try to fix something that is almost impossible. ;-) I have a working fix now. It's similar to the previous hack of EventQueue, but it operates on the existing EventQueue rather then creating a new one. It assures that while our dialog is opened, only selected events gets processed. I do not consider it as a solution of the original problem, but just as a fix of deadlocks of this kind. The original problem must be solved together with issue #57748. Created attachment 21674 [details]
The working fix to this deadlock.
I've put the fix into trunk so that it can be tested. The behavior is not great (the dialogs deserve some improvement), but it does not deadlock: /cvs/vcscore/src/org/netbeans/modules/vcscore/commands/DialogVisualizerWrapper.java,v <-- DialogVisualizerWrapper.java new revision: 1.2; previous revision: 1.1 The fix is potentially dangerous, therefore I think that it should not go into release41 without proper testing. It can be considered for hotfixes if it proves to work fine... I agree this would best be waived for 4.1, at least until it is more thoroughly studied and tested. No objections to the waiver request for 4.1 ==> approved |