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: | Deadlock caused by double locking in AbstractModel (synchronized methods and transactionSemaphore) | ||
---|---|---|---|
Product: | xml | Reporter: | Tomas Danek <musilt2> |
Component: | XAM | Assignee: | Antonin Nebuzelsky <anebuzelsky> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | anebuzelsky, jglick, mmirilovic, pjiricka, supernikita |
Priority: | P1 | Keywords: | THREAD |
Version: | 7.0 | ||
Hardware: | PC | ||
OS: | Mac OS X | ||
Issue Type: | DEFECT | Exception Reporter: | |
Attachments: |
thread dump
modified source file |
Description
Tomas Danek
2010-11-10 13:09:43 UTC
Created attachment 102883 [details]
thread dump
Please evaluate for potential beta stopper, thanks. Deadlock between "StatusProvider" and "AWT-EventQueue-1". AbstractModel.endTransaction() is waiting to enter its synchronized block and the lock is held by AbstractModel.startTransaction() in the other thread (AWT) which is blocked inside this method in transactionSemaphore.acquireUninterruptibly(). AWT thread will be able to acquire only after the transaction semaphore is released (which would happen in endTransaction if it was not blocked). Why is there the double locking in AbstractModel? 1. synchronized access to startTransaction() and endTransaction() 2. transactionSemaphore Reassigning to xml. IIRC there is a duplicate of this somewhere in the xml component. *** Bug 190532 has been marked as a duplicate of this bug. *** Ok. Looks like but is not constantly reproducible. However, Nikita please take a look. And yes, it can be related to Maven plugin. > Why is there the double locking in AbstractModel?
>
> 1. synchronized access to startTransaction() and endTransaction()
> 2. transactionSemaphore
>
I've looked the sources and I agree with your explanation. I wonder how it managed to work so far for about 4 years. Maybe because there wasn't many multi-thread usage of XAM model.
I think it's necessary to get rid of transactionSemaphore by replacing it with wait() & notify() calls. Despite of Semaphore, they are consolidated with synchronized clause. So result should be similar, but it should prevent the deadlock.
Unfortunately true history of relevant modifications has lost. They have been done about 4 years ago. But I suppose the reason of Semaphore usage is explained partly at the comment at lines 405-408. As I understood, they wanted avoiding reentrant transactions (by the same thread). But now it seems impossible because of the check at the beginning of startTransaction() method. So replacement to wait¬ify shouldn't break this feature.
It seems can be fixed quite quickly. But any change to XAM model is very dangerous because it can affect a lot of other modules. So it requires a lot of time on testing. I'm out of NetBeans development process now (and all my team as well). So I'd like either somebody else does the fix or at least I need get help from somebody about repository, release schedule and so on.
I'm going to prepare changes, which I've mentioned. I suppose they all will be located at single file: AbstractModel.java Then I can test it quickly locally and also can send it (source file) to maven team for testing. I think the issue shouldn't be easy to reproduce like any other problems with threads synchronization. But I hope you'll be able to make any assessment. Created attachment 102989 [details]
modified source file
You can find the suggested modifications in the source file.
I've compiled it (and NetBeans at all), run JUnit tests for several modules and run NetBeans with it.
I haven't faced any problems except many errors in BPEL Model JUnit tests. But then it turned out that absolutely the same errors happen without my modification. So it's another problem.
Tonda do you want to test the proposed change? Thanks for the patch! It looks correct to me. Integrated. I only changed wait(1000) to wait(). http://hg.netbeans.org/core-main/rev/4f5854f34dd1 This fix is not in 7.0 Beta, to be released soon. It will appear only in daily builds, starting with the next one. *** Bug 192037 has been marked as a duplicate of this bug. *** Integrated into 'main-golden', will be available in build *201011190001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main/rev/4f5854f34dd1 User: Antonin Nebuzelsky <anebuzelsky@netbeans.org> Log: #191796 - Deadlock caused by double locking in AbstractModel (synchronized methods and transactionSemaphore). *** Bug 184371 has been marked as a duplicate of this bug. *** |