[Jython-checkins] jython: Avoid deadlock in running closers. Fixes #2457
jim.baker
jython-checkins at python.org
Wed Feb 3 20:26:58 EST 2016
https://hg.python.org/jython/rev/94b447b00840
changeset: 7895:94b447b00840
user: Jim Baker <jim.baker at rackspace.com>
date: Wed Feb 03 18:26:52 2016 -0700
summary:
Avoid deadlock in running closers. Fixes #2457
This is a further elaboration of the work done in #2280, and should be
completely compatible (and has been tested) with the recent change in
r7889:cddadb6808c0.
Jim's theory (and Jim is the committer of this change on Nick's
behalf): synchronizing on the class, vs an object, is a bad idea
because of class unloading that's happening at shutdown. At the very
least this is now tied to the closer itself.
files:
src/org/python/core/PySystemState.java | 99 ++++-----
src/org/python/core/finalization/FinalizeTrigger.java | 32 +-
2 files changed, 64 insertions(+), 67 deletions(-)
diff --git a/src/org/python/core/PySystemState.java b/src/org/python/core/PySystemState.java
--- a/src/org/python/core/PySystemState.java
+++ b/src/org/python/core/PySystemState.java
@@ -17,6 +17,7 @@
import java.nio.charset.Charset;
import java.nio.charset.UnsupportedCharsetException;
import java.security.AccessControlException;
+import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashSet;
@@ -1507,7 +1508,7 @@
public static class PySystemStateCloser {
- private final Set<Callable<Void>> resourceClosers = new LinkedHashSet<Callable<Void>>();
+ private final Set<Callable<Void>> resourceClosers = Collections.synchronizedSet(new LinkedHashSet<Callable<Void>>());
private volatile boolean isCleanup = false;
private final Thread shutdownHook;
@@ -1528,63 +1529,55 @@
}
private void registerCloser(Callable<Void> closer) {
- synchronized (PySystemStateCloser.class) {
- if (!isCleanup) {
- resourceClosers.add(closer);
- }
+ if (!isCleanup) {
+ resourceClosers.add(closer);
}
}
private boolean unregisterCloser(Callable<Void> closer) {
- synchronized (PySystemStateCloser.class) {
- return resourceClosers.remove(closer);
- }
+ return resourceClosers.remove(closer);
}
private synchronized void cleanup() {
- synchronized (PySystemStateCloser.class) {
- if (isCleanup) {
- return;
+ if (isCleanup) {
+ return;
+ }
+ isCleanup = true;
+
+ // close this thread so we can unload any associated classloaders in cycle
+ // with this instance
+ if (shutdownHook != null) {
+ try {
+ Runtime.getRuntime().removeShutdownHook(shutdownHook);
+ } catch (IllegalStateException e) {
+ // JVM is already shutting down, so we cannot remove this shutdown hook anyway
}
- isCleanup = true;
+ }
- // close this thread so we can unload any associated classloaders in cycle
- // with this instance
- if (shutdownHook != null) {
+ // Close the listed resources (and clear the list)
+ runClosers();
+ resourceClosers.clear();
+
+ // XXX Not sure this is ok, but it makes repeat testing possible.
+ // Re-enable the management of resource closers
+ isCleanup = false;
+ }
+ private synchronized void runClosers() {
+ // resourceClosers can be null in some strange cases
+ if (resourceClosers != null) {
+ /*
+ * Although a Set, the container iterates in the order closers were added. Make a Deque
+ * of it and deal from the top.
+ */
+ LinkedList<Callable<Void>> rc = new LinkedList<Callable<Void>>(resourceClosers);
+ Iterator<Callable<Void>> iter = rc.descendingIterator();
+
+ while (iter.hasNext()) {
+ Callable<Void> callable = iter.next();
try {
- Runtime.getRuntime().removeShutdownHook(shutdownHook);
- } catch (IllegalStateException e) {
- // JVM is already shutting down, so we cannot remove this shutdown hook anyway
- }
- }
-
- // Close the listed resources (and clear the list)
- runClosers();
- resourceClosers.clear();
-
- // XXX Not sure this is ok, but it makes repeat testing possible.
- // Re-enable the management of resource closers
- isCleanup = false;
- }
- }
- private void runClosers() {
- synchronized (PySystemStateCloser.class) {
- // resourceClosers can be null in some strange cases
- if (resourceClosers != null) {
- /*
- * Although a Set, the container iterates in the order closers were added. Make a Deque
- * of it and deal from the top.
- */
- LinkedList<Callable<Void>> rc = new LinkedList<Callable<Void>>(resourceClosers);
- Iterator<Callable<Void>> iter = rc.descendingIterator();
-
- while (iter.hasNext()) {
- Callable<Void> callable = iter.next();
- try {
- callable.call();
- } catch (Exception e) {
- // just continue, nothing we can do
- }
+ callable.call();
+ } catch (Exception e) {
+ // just continue, nothing we can do
}
}
}
@@ -1593,7 +1586,7 @@
// Python scripts expect that files are closed upon an orderly cleanup of the VM.
private Thread initShutdownCloser() {
try {
- Thread shutdownHook = new Thread(new ShutdownCloser(), "Jython Shutdown Closer");
+ Thread shutdownHook = new Thread(new ShutdownCloser(this), "Jython Shutdown Closer");
Runtime.getRuntime().addShutdownHook(shutdownHook);
return shutdownHook;
} catch (SecurityException se) {
@@ -1603,10 +1596,16 @@
}
private class ShutdownCloser implements Runnable {
+ PySystemStateCloser closer = null;
+
+ public ShutdownCloser(PySystemStateCloser closer) {
+ super();
+ this.closer = closer;
+ }
@Override
public void run() {
- synchronized (PySystemStateCloser.class) {
+ synchronized (this.closer) {
runClosers();
resourceClosers.clear();
}
diff --git a/src/org/python/core/finalization/FinalizeTrigger.java b/src/org/python/core/finalization/FinalizeTrigger.java
--- a/src/org/python/core/finalization/FinalizeTrigger.java
+++ b/src/org/python/core/finalization/FinalizeTrigger.java
@@ -76,25 +76,23 @@
}
public static void runFinalizer(PyObject toFinalize, boolean runBuiltinOnly) {
- synchronized (PySystemState.PySystemStateCloser.class) {
- if (!runBuiltinOnly) {
- if (toFinalize instanceof FinalizablePyObjectDerived) {
- try {
- ((FinalizablePyObjectDerived) toFinalize).__del_derived__();
- } catch (Exception e) {
- }
- } else if (toFinalize instanceof FinalizablePyObject) {
- try {
- ((FinalizablePyObject) toFinalize).__del__();
- } catch (Exception e) {
- }
+ if (!runBuiltinOnly) {
+ if (toFinalize instanceof FinalizablePyObjectDerived) {
+ try {
+ ((FinalizablePyObjectDerived) toFinalize).__del_derived__();
+ } catch (Exception e) {
+ }
+ } else if (toFinalize instanceof FinalizablePyObject) {
+ try {
+ ((FinalizablePyObject) toFinalize).__del__();
+ } catch (Exception e) {
}
}
- if (toFinalize instanceof FinalizableBuiltin) {
- try {
- ((FinalizableBuiltin) toFinalize).__del_builtin__();
- } catch (Exception e) {
- }
+ }
+ if (toFinalize instanceof FinalizableBuiltin) {
+ try {
+ ((FinalizableBuiltin) toFinalize).__del_builtin__();
+ } catch (Exception e) {
}
}
}
--
Repository URL: https://hg.python.org/jython
More information about the Jython-checkins
mailing list