diff --git a/masterfs/src/org/netbeans/modules/masterfs/filebasedfs/utils/FileChangedManager.java b/masterfs/src/org/netbeans/modules/masterfs/filebasedfs/utils/FileChangedManager.java --- a/masterfs/src/org/netbeans/modules/masterfs/filebasedfs/utils/FileChangedManager.java +++ b/masterfs/src/org/netbeans/modules/masterfs/filebasedfs/utils/FileChangedManager.java @@ -183,6 +183,35 @@ return IDLE_IO.get() != null; } + /** + * If invoked in {@link #idleIO(int, java.lang.Runnable, + * java.lang.Runnable, java.util.concurrent.atomic.AtomicBoolean) idleIO}, + * wait until all priority IOs are completed and start {@link Runnable} + * {@code run}. The {@code run} will not be blocked in FileChangedManager + * even if another priority IO is started (this would cause deadlocks, see + * bug 246893). + */ + public static void waitNowAndRun(Runnable run) { + Integer storedMaxLoad = IDLE_IO.get(); + if (storedMaxLoad != null) { + try { + waitIOLoadLowerThan(storedMaxLoad); + } catch (InterruptedException ex) { + if (isFine) { + LOG.log(Level.FINE, "Interrupted {0}", ex.getMessage()); + } + } + IDLE_IO.set(null); + try { + run.run(); + } finally { + IDLE_IO.set(storedMaxLoad); + } + } else { + run.run(); + } + } + public static void idleIO(int maximumLoad, Runnable r, Runnable goingToSleep, AtomicBoolean goOn) { Integer prev = IDLE_IO.get(); Runnable pGoing = IDLE_CALL.get(); diff --git a/masterfs/src/org/netbeans/modules/masterfs/watcher/Watcher.java b/masterfs/src/org/netbeans/modules/masterfs/watcher/Watcher.java --- a/masterfs/src/org/netbeans/modules/masterfs/watcher/Watcher.java +++ b/masterfs/src/org/netbeans/modules/masterfs/watcher/Watcher.java @@ -52,6 +52,7 @@ import java.util.logging.Level; import java.util.logging.Logger; import org.netbeans.modules.masterfs.filebasedfs.fileobjects.FileObjectFactory; +import org.netbeans.modules.masterfs.filebasedfs.utils.FileChangedManager; import org.netbeans.modules.masterfs.providers.InterceptionListener; import org.netbeans.modules.masterfs.providers.ProvidedExtensions; import org.openide.filesystems.FileObject; @@ -225,7 +226,7 @@ } } - final void register(FileObject fo) { + final void register(final FileObject fo) { if (fo.isValid() && !fo.isFolder()) { LOG.log(Level.INFO, "Should be a folder: {0} data: {1} folder: {2} valid: {3}", new Object[]{fo, fo.isData(), fo.isFolder(), fo.isValid()}); } @@ -234,6 +235,20 @@ } catch (IOException ex) { LOG.log(Level.INFO, "Exception while clearing the queue", ex); } + FileChangedManager.waitNowAndRun(new Runnable() { + @Override + public void run() { + registerSynchronized(fo); + } + }); + } + + /** + * Part of registration process that cannot be blocked by + * FileChangedManager, and which can take lock LOCK. See bug 246893. + * (FileChangedManager's "lock" must be taken always first.) + */ + private void registerSynchronized(FileObject fo) { synchronized (LOCK) { NotifierKeyRef kr = new NotifierKeyRef(fo, null, null, impl); if (getReferences().contains(kr)) { diff --git a/masterfs/test/unit/src/org/netbeans/modules/masterfs/filebasedfs/utils/FileChangedManagerDeadlockTest.java b/masterfs/test/unit/src/org/netbeans/modules/masterfs/filebasedfs/utils/FileChangedManagerDeadlockTest.java new file mode 100644 --- /dev/null +++ b/masterfs/test/unit/src/org/netbeans/modules/masterfs/filebasedfs/utils/FileChangedManagerDeadlockTest.java @@ -0,0 +1,223 @@ +/* + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER. + * + * Copyright 2014 Oracle and/or its affiliates. All rights reserved. + * + * Oracle and Java are registered trademarks of Oracle and/or its affiliates. + * Other names may be trademarks of their respective owners. + * + * The contents of this file are subject to the terms of either the GNU + * General Public License Version 2 only ("GPL") or the Common + * Development and Distribution License("CDDL") (collectively, the + * "License"). You may not use this file except in compliance with the + * License. You can obtain a copy of the License at + * http://www.netbeans.org/cddl-gplv2.html + * or nbbuild/licenses/CDDL-GPL-2-CP. See the License for the + * specific language governing permissions and limitations under the + * License. When distributing the software, include this License Header + * Notice in each file and include the License file at + * nbbuild/licenses/CDDL-GPL-2-CP. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the GPL Version 2 section of the License file that + * accompanied this code. If applicable, add the following below the + * License Header, with the fields enclosed by brackets [] replaced by + * your own identifying information: + * "Portions Copyrighted [year] [name of copyright owner]" + * + * If you wish your version of this file to be governed by only the CDDL + * or only the GPL Version 2, indicate your decision by adding + * "[Contributor] elects to include this software in this distribution + * under the [CDDL or GPL Version 2] license." If you do not indicate a + * single choice of license, a recipient has the option to distribute + * your version of this file under either the CDDL, the GPL Version 2 or + * to extend the choice of license to its licensees as provided above. + * However, if you add GPL Version 2 code and therefore, elected the GPL + * Version 2 license, then the option applies only if the new code is + * made subject to such option by the copyright holder. + * + * Contributor(s): + * + * Portions Copyrighted 2014 Sun Microsystems, Inc. + */ +package org.netbeans.modules.masterfs.filebasedfs.utils; + +import java.io.File; +import java.io.IOException; +import java.util.concurrent.Callable; +import java.util.concurrent.Semaphore; +import java.util.concurrent.atomic.AtomicBoolean; +import static junit.framework.Assert.assertTrue; +import org.junit.Before; +import org.junit.Test; +import org.netbeans.junit.MockServices; +import org.netbeans.modules.masterfs.providers.Notifier; +import org.netbeans.modules.masterfs.watcher.Watcher; +import org.openide.filesystems.FileUtil; +import org.openide.util.Exceptions; +import org.openide.util.Lookup; + +/** + * + * @author jhavlin + */ +public class FileChangedManagerDeadlockTest { + + @Before + public void setUp() { + MockServices.setServices(MockNotifier.class); + } + + @Test + public void testDeadlockWhenNotifierCallsSecurityManager() throws + InterruptedException { + + // permits - progress + // 1 - Priority IO can enter priority section (lock taken by idle IO) + // 2 - Idle IO can ask for some IO operation (security manager blocked) + Semaphore s = new Semaphore(0); + + Lookup.getDefault().lookup(MockNotifier.class).setSemaphore(s); + + IdleIO idleIO = new IdleIO(s); + PriorityIO priorityIO = new PriorityIO(s); + + // Thread 1 - Enter idleIO, add a watch -> wait for priority + Thread idle = new Thread(idleIO, "Idle IO Thread"); + + // Thread 2 - Enter priority IO, wait for watch's lock. + Thread priority = new Thread(priorityIO, "Priority IO Thread"); + + idle.start(); + priority.start(); + + idle.join(2000); + priority.join(2000); + + assertTrue("Idle IO should be finished", idleIO.finished.get()); + assertTrue("Priority IO should be finished", priorityIO.finished.get()); + } + + private static abstract class IORunnable implements Runnable { + + protected Semaphore semaphore; + final AtomicBoolean finished = new AtomicBoolean(false); + + public IORunnable(Semaphore semaphore) { + this.semaphore = semaphore; + } + + @Override + public void run() { + try { + runInner(); + } catch (Exception ex) { + Exceptions.printStackTrace(ex); + } + finished.set(true); + } + + public abstract void runInner() throws Exception; + + public void addSomeWatch() { + String homeDir = System.getProperty("user.home"); + Watcher.register(FileUtil.toFileObject( + new File(homeDir).getAbsoluteFile())); + } + } + + private static class IdleIO extends IORunnable { + + public IdleIO(Semaphore semaphore) { + super(semaphore); + } + + @Override + public void runInner() { + FileChangedManager.idleIO(50, new Runnable() { + + @Override + public void run() { + // Add watch in idle thread. This cannot finish, because + // it takes Watcher lock and then waits for the priority + // thread, which waits for the watcher lock. + addSomeWatch(); + } + }, null, new AtomicBoolean(true)); + } + } + + private static class PriorityIO extends IORunnable { + + public PriorityIO(Semaphore semaphore) { + super(semaphore); + } + + @Override + public void runInner() throws Exception { + + // Start priority thread after the lock is taken by the idle thread. + semaphore.acquire(1); + FileChangedManager.priorityIO(new Callable() { + @Override + public Boolean call() throws Exception { + // Continue the idle thread. FileChangedManager will now + // wait for this priority thread to complete. + semaphore.release(2); + // But this thread cannot complete, because it needs the + // watcher lock which is held by the idle thread. + addSomeWatch(); + return true; + } + }); + } + } + + @SuppressWarnings("PublicInnerClass") + public static class MockNotifier extends Notifier { + + private Semaphore semaphore = null; + + public void setSemaphore(Semaphore semaphore) { + this.semaphore = semaphore; + } + + /** + * Simulate a notifier that invokes SecurityManager when adding a watch. + * + * @param path + * @return + * @throws IOException + */ + @Override + protected String addWatch(String path) throws IOException { + if (semaphore != null) { + // Idle thread has watcher lock. Priority thread can start. + semaphore.release(1); + } + try { + // Priority thread has entered the priority section. Idle thread + // can continue. + semaphore.acquire(2); + } catch (InterruptedException ex) { + Exceptions.printStackTrace(ex); + } + FileChangedManager.getInstance().checkRead(path); + return "path"; + } + + @Override + protected void removeWatch(String key) throws IOException { + } + + @Override + protected synchronized String nextEvent() throws IOException, + InterruptedException { + wait(); // wait forever + return null; + } + + @Override + protected void start() throws IOException { + } + } +}