Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8238169

BasicDirectoryModel getDirectories and DoChangeContents.run can deadlock





        Report by Adrian Nistor <adrnisto@amazon.com>.

        The code in BasicDirectoryModel.java acquires synchronization objects in different orders. One acquires DoChangeContents.this and then fileCache, the other acquires fileCache and then DoChangeContents.this.

        Acquiring synchronization objects in different orders can cause a circular wait (deadlock).

        Here is the first order: (1) first acquire DoChangeContents.this and then fileCache:

            enter public method run() at line 524
            acquire synchronization on DoChangeContents.this at entry to run() because run() is a synchronized method --- see line 524
            acquire synchronization on fileCache at line 528

        Here is the second order: (2) first acquire fileCache and then DoChangeContents.this:

            enter public method renameFile() at line 176
            acquire synchronization on fileCache at line 177
            call validateFileCache() at line 179
            call cancelRunnables() at line 157
            call cancelRunnables(Vector) at line 397
            call cancel() at line 392
            acquire synchronization on DoChangeContents.this at entry to cancel() because cancel() is a synchronized method --- see line 520

        A straight-forward fix always acquires synchronization in only one order: first fileCache and then DoChangeContents.this. Swap the order of synchronized (this) and synchronized (fileCache) at lines 524 and 528.

        Removing the synchronized keyword from the method signature and replacing it with synchronized (this) ensures the same behavior.

        public void run() { <<<<<<<<<<<<<<<<<<<<<<<<<<<<< removed synchronized from here and moved it to synchronized (this) 2 lines below
            synchronized(fileCache) { <<<<<<<<<<<<<<<<<<<<<<< added this statement instead of the same statement 5 lines below
                synchronized(this) { <<<<<<<<<<<<<<<<<<<<< added this instead of the synchronized keyword in the method signature 2 lines above
                    if (fetchID == fid && doFire) {
                        int remSize = (remFiles == null) ? 0 : remFiles.size();
                        int addSize = (addFiles == null) ? 0 : addFiles.size();
                        synchronized(fileCache) { <<<<<<<<<<<<<<<< ***remove*** from here and moved it 5 lines above
        ... ... ... ... ... ... ... ...

        The downside is that the fileCache synchronized region is larger than before.


          Issue Links



                aivanov Alexey Ivanov
                phh Paul Hohensee
                0 Vote for this issue
                6 Start watching this issue