Permalink
Browse files

Final fixes and test for JRUBY-4264: threadContextMap leaks RubyThrea…

…d on death of adopted thread
  • Loading branch information...
1 parent 3a3c8cd commit fffa5a791b4a27f83ea4809fcef02501ceb2cc3e @headius headius committed May 2, 2010
@@ -33,6 +33,7 @@
package org.jruby;
import java.io.IOException;
+import java.lang.ref.WeakReference;
import java.nio.channels.Channel;
import java.nio.channels.SelectableChannel;
import java.nio.channels.SelectionKey;
@@ -90,6 +91,9 @@
// Error info is per-thread
private IRubyObject errorInfo;
+
+ // weak reference to associated ThreadContext
+ private volatile WeakReference<ThreadContext> contextRef;
private static final boolean DEBUG = false;
@@ -154,6 +158,14 @@ public IRubyObject setErrorInfo(IRubyObject errorInfo) {
this.errorInfo = errorInfo;
return errorInfo;
}
+
+ public void setContext(ThreadContext context) {
+ this.contextRef = new WeakReference<ThreadContext>(context);
+ }
+
+ public ThreadContext getContext() {
+ return contextRef.get();
+ }
/**
* Dispose of the current thread by removing it from its parent ThreadGroup.
@@ -1,124 +0,0 @@
-/***** BEGIN LICENSE BLOCK *****
- * Version: CPL 1.0/GPL 2.0/LGPL 2.1
- *
- * The contents of this file are subject to the Common Public
- * License Version 1.0 (the "License"); you may not use this file
- * except in compliance with the License. You may obtain a copy of
- * the License at http://www.eclipse.org/legal/cpl-v10.html
- *
- * Software distributed under the License is distributed on an "AS
- * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
- * implied. See the License for the specific language governing
- * rights and limitations under the License.
- *
- * Copyright (C) 2010 Charles O Nutter <headius@headius.com>
- *
- * Alternatively, the contents of this file may be used under the terms of
- * either of the GNU General Public License Version 2 or later (the "GPL"),
- * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
- * in which case the provisions of the GPL or the LGPL are applicable instead
- * of those above. If you wish to allow use of your version of this file only
- * under the terms of either the GPL or the LGPL, and not to allow others to
- * use your version of this file under the terms of the CPL, indicate your
- * decision by deleting the provisions above and replace them with the notice
- * and other provisions required by the GPL or the LGPL. If you do not delete
- * the provisions above, a recipient may use your version of this file under
- * the terms of any one of the CPL, the GPL or the LGPL.
- ***** END LICENSE BLOCK *****/
-package org.jruby.internal.runtime;
-
-import java.lang.ref.ReferenceQueue;
-import java.lang.ref.WeakReference;
-import java.util.Hashtable;
-import java.util.Map;
-import java.util.Set;
-import org.jruby.RubyThread;
-import org.jruby.runtime.ThreadContext;
-
-public class RubyThreadMap {
- /** For null keys, to mimic WeakHashMap */
- private static final Object NULL_KEY = new Object();
-
- private final Map<RubyThreadWeakReference<Object>, RubyThread> map = new Hashtable();
- private final ReferenceQueue<Object> queue = new ReferenceQueue();
- private final Map<RubyThread, ThreadContext> mapToClean;
-
- public RubyThreadMap(Map<RubyThread, ThreadContext> mapToClean) {
- this.mapToClean = mapToClean;
- }
-
- public static class RubyThreadWeakReference<T> extends WeakReference<T> {
- private final RubyThread thread;
- public int hashCode;
- public RubyThreadWeakReference(T referrent, RubyThread thread) {
- super(referrent);
- hashCode = referrent.hashCode();
- this.thread = thread;
- }
- public RubyThreadWeakReference(T referrent, ReferenceQueue<? super T> queue, RubyThread thread) {
- super(referrent, queue);
- hashCode = referrent.hashCode();
- this.thread = thread;
- }
- public RubyThread getThread() {
- return thread;
- }
- @Override
- public int hashCode() {
- return hashCode;
- }
- @Override
- public boolean equals(Object other) {
- Object myKey = get();
- if (other instanceof RubyThreadWeakReference) {
- Object otherKey = ((RubyThreadWeakReference)other).get();
- if (myKey != otherKey) return false;
- return true;
- } else if (other instanceof Thread) {
- return myKey == other;
- } else {
- return false;
- }
- }
- }
-
- private void cleanup() {
- RubyThreadWeakReference<Object> ref;
- while ((ref = (RubyThreadWeakReference<Object>) queue.poll()) != null) {
- map.remove(ref);
- mapToClean.remove(ref.getThread());
- }
- }
-
- public int size() {
- cleanup();
- return map.size();
- }
-
- public Set<Map.Entry<RubyThreadWeakReference<Object>, RubyThread>> entrySet() {
- return map.entrySet();
- }
-
- public RubyThread get(Object key) {
- cleanup();
- key = nullKey(key);
- return map.get(key);
- }
-
- public RubyThread put(Object key, RubyThread value) {
- cleanup();
- key = nullKey(key);
- return map.put(new RubyThreadWeakReference(key, queue, value), value);
- }
-
- public RubyThread remove(Object key) {
- cleanup();
- key = nullKey(key);
- RubyThread t = map.remove(key);
- return t;
- }
-
- private Object nullKey(Object key) {
- return key == null ? NULL_KEY : key;
- }
-}
@@ -45,16 +45,95 @@
import org.jruby.runtime.builtin.IRubyObject;
import org.jruby.runtime.ThreadContext;
+/**
+ * ThreadService maintains lists ofall the JRuby-specific thread data structures
+ * needed for Ruby's threading API and for JRuby's execution. The main
+ * structures are:
+ *
+ * <ul>
+ * <li>ThreadContext, which contains frames, scopes, etc needed for Ruby execution</li>
+ * <li>RubyThread, the Ruby object representation of a thread's state</li>
+ * <li>RubyThreadGroup, which represents a group of Ruby threads</li>
+ * </ul>
+ *
+ * In order to ensure these structures do not linger after the thread has terminated,
+ * most of them are either weakly or softly referenced. The references associated
+ * with these structures are:
+ *
+ * <ul>
+ * <li>ThreadService has a hard reference to a ThreadLocal, which holds a soft reference
+ * to a ThreadContext. So the thread's locals softly reference ThreadContext.
+ * We use a soft reference to keep ThreadContext instances from going away too
+ * quickly when a Java thread leaves Ruby space completely, which would otherwise
+ * result in a lot of ThreadContext object churn.</li>
+ * <li>ThreadService maintains a weak map from the actual java.lang.Thread (or
+ * java.util.concurrent.Future) instance to the associated RubyThread. @see org.jruby.internal.runtime.RubyThreadMap.
+ * Because this map is weak-valued rather than weak-keyed, it may hold Thread/Future
+ * references beyond their useful lifetime, but they will get cleaned when the
+ * map is accessed again.</li>
+ * <li>RubyThread has a weak reference to its to ThreadContext.</li>
+ * <li>ThreadContext has a hard reference to its associated RubyThread. Ignoring other
+ * resources, this will usually mean RubyThread is softly reachable via the
+ * soft threadlocal reference to ThreadContext in ThreadService, but weakly
+ * referenced otherwise.</li>
+ * <li>RubyThreadGroup has hard references to threads it owns. The thread removes
+ * itself on termination (if it's a Ruby thread) or when the ThreadContext is
+ * collected (as in the case of "adopted" Java threads.</li>
+ * </ul>
+ *
+ * These data structures can come to life in one of two ways:
+ *
+ * <ul>
+ * <li>A Ruby thread is started. This constructs a new RubyThread object, which
+ * calls to ThreadService to initialize a ThreadContext and appropriate mappings
+ * in all ThreadService's structures. The body of the thread is wrapped with a
+ * finally block that will forcibly unregister the thread and all related
+ * structures from ThreadService.</li>
+ * <li>A Java thread enters Ruby by doing a call. The thread is "adopted", and
+ * gains a RubyThread instance, a ThreadContext instance, and all associated
+ * mappings in ThreadService. Since we don't know when the thread has "left"
+ * Ruby permanently, no forcible unregistration is attempted for the various
+ * structures and maps. However, they should not be hard-rooted; the
+ * ThreadContext is only softly reachable at best if no calls are in-flight,
+ * so it will collect. Its collection will release the reference to RubyThread,
+ * and its finalizer will unregister that RubyThread from its RubyThreadGroup.
+ * With the RubyThread gone, the Thread-to-RubyThread map will eventually clear,
+ * releasing the hard reference to the Thread itself.</li>
+ * <ul>
+ */
public class ThreadService {
private Ruby runtime;
+ /**
+ * A hard reference to the "main" context, so we always have one waiting for
+ * "main" thread execution.
+ */
private ThreadContext mainContext;
+
+ /**
+ * A thread-local soft reference to the current thread's ThreadContext. We
+ * use a soft reference so that the ThreadContext is still collectible but
+ * will not immediately disappear once dereferenced, to avoid churning
+ * through ThreadContext instances every time a Java thread enters and exits
+ * Ruby space.
+ */
private ThreadLocal<SoftReference<ThreadContext>> localContext;
+
+ /**
+ * The Java thread group into which we register all Ruby threads. This is
+ * distinct from the RubyThreadGroup, which is simply a mutable collection
+ * of threads.
+ */
private ThreadGroup rubyThreadGroup;
- private RubyThreadMap rubyThreadMap;
- private Map<RubyThread,ThreadContext> threadContextMap;
+ /**
+ * A map from a Java Thread or Future to its RubyThread instance. This map
+ * is weak-valued, so it will not prevent collection of the RubyThread. It
+ * lazily cleans out dead entries, so it may hold references to Threads or
+ * Futures longer then their useful lifetime.
+ */
+ private final Map<Object, RubyThread> rubyThreadMap;
- private ReentrantLock criticalLock = new ReentrantLock();
+ private final ReentrantLock criticalLock = new ReentrantLock();
public ThreadService(Ruby runtime) {
this.runtime = runtime;
@@ -67,16 +146,13 @@ public ThreadService(Ruby runtime) {
this.rubyThreadGroup = Thread.currentThread().getThreadGroup();
}
- this.threadContextMap = Collections.synchronizedMap(new WeakHashMap<RubyThread, ThreadContext>());
- this.rubyThreadMap = new RubyThreadMap(threadContextMap);
+ this.rubyThreadMap = Collections.synchronizedMap(new WeakHashMap<Object, RubyThread>());
// Must be called from main thread (it is currently, but this bothers me)
localContext.set(new SoftReference<ThreadContext>(mainContext));
}
public void disposeCurrentThread() {
- RubyThread thread = getCurrentContext().getThread();
- threadContextMap.remove(thread);
localContext.set(null);
rubyThreadMap.remove(Thread.currentThread());
}
@@ -140,7 +216,6 @@ public RubyThread getMainThread() {
public void setMainThread(Thread thread, RubyThread rubyThread) {
mainContext.setThread(rubyThread);
- threadContextMap.put(rubyThread, mainContext);
rubyThreadMap.put(thread, rubyThread);
}
@@ -150,8 +225,8 @@ public void setMainThread(Thread thread, RubyThread rubyThread) {
synchronized(rubyThreadMap) {
List<RubyThread> rtList = new ArrayList<RubyThread>(rubyThreadMap.size());
- for (Map.Entry<RubyThreadMap.RubyThreadWeakReference<Object>, RubyThread> entry : rubyThreadMap.entrySet()) {
- Object key = entry.getKey().get();
+ for (Map.Entry<Object, RubyThread> entry : rubyThreadMap.entrySet()) {
+ Object key = entry.getKey();
if (key == null) continue;
if (key instanceof Thread) {
@@ -181,13 +256,12 @@ public ThreadGroup getRubyThreadGroup() {
}
public ThreadContext getThreadContextForThread(RubyThread thread) {
- return threadContextMap.get(thread);
+ return thread.getContext();
}
public synchronized ThreadContext registerNewThread(RubyThread thread) {
ThreadContext context = ThreadContext.newContext(runtime);
localContext.set(new SoftReference(context));
- threadContextMap.put(thread, context);
context.setThread(thread);
return context;
}
@@ -202,7 +276,6 @@ public synchronized void dissociateThread(Object threadOrFuture) {
public synchronized void unregisterThread(RubyThread thread) {
rubyThreadMap.remove(Thread.currentThread());
- threadContextMap.remove(thread);
getCurrentContext().setThread(null);
localContext.set(null);
}
@@ -252,18 +325,7 @@ public synchronized void deliverEvent(Event event) {
*
* @return The ruby thread map
*/
- public RubyThreadMap getRubyThreadMap() {
+ public Map<Object, RubyThread> getRubyThreadMap() {
return rubyThreadMap;
}
-
- /**
- * Get the map from RubyThread objects to ThreadContext objects. Used for
- * testing purposes.
- *
- * @return The thread context map
- * @return
- */
- public Map getThreadContextMap() {
- return threadContextMap;
- }
}
@@ -255,6 +255,11 @@ public RubyThread getThread() {
public void setThread(RubyThread thread) {
this.thread = thread;
+
+ // associate the thread with this context, unless we're clearing the reference
+ if (thread != null) {
+ thread.setContext(this);
+ }
}
public Fiber getFiber() {
View
@@ -122,3 +122,4 @@ test_weak_drb_id_conv
test_kernel
test_dir_with_plusses
test_jar_file
+test_thread_service
Oops, something went wrong.

0 comments on commit fffa5a7

Please sign in to comment.