Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixes for test_thread in thread pooling mode

* Allow setting priorities for pooled threads
* Aggregate "dispose" functionality for threads
* Reset pooled thread priorities to initial
* Remove undefined-behavior "dead thread" priority test
* Make Thread.current keys test permissive about extra keys
  • Loading branch information...
commit affd4d8a90b84e4d0adb47228b30ff2305715dc6 1 parent 3676f3f
@headius headius authored
View
53 src/org/jruby/RubyThread.java
@@ -67,6 +67,8 @@
import org.jruby.runtime.ClassIndex;
import org.jruby.runtime.ObjectMarshal;
import static org.jruby.runtime.Visibility.*;
+
+import org.jruby.util.cli.Options;
import org.jruby.util.io.BlockingIO;
import org.jruby.util.io.SelectorFactory;
import org.jruby.util.log.Logger;
@@ -138,6 +140,12 @@
/** The list of locks this thread currently holds, so they can be released on exit */
private final List<Lock> heldLocks = new ArrayList<Lock>();
+ /** Whether or not this thread has been disposed of */
+ private volatile boolean disposed = false;
+
+ /** The thread's initial priority, for use in thread pooled mode */
+ private int initialPriority;
+
protected RubyThread(Ruby runtime, RubyClass type) {
super(runtime, type);
@@ -207,11 +215,46 @@ public ThreadContext getContext() {
public Thread getNativeThread() {
return threadImpl.nativeThread();
}
+
/**
- * Dispose of the current thread by removing it from its parent ThreadGroup.
+ * Perform pre-execution tasks once the native thread is running, but we
+ * have not yet called the Ruby code for the thread.
*/
- public void dispose() {
- threadGroup.remove(this);
+ public void beforeStart() {
+ // store initial priority, for restoring pooled threads to normal
+ initialPriority = threadImpl.getPriority();
+
+ // set to "normal" priority
+ threadImpl.setPriority(Thread.NORM_PRIORITY);
+ }
+
+ /**
+ * Dispose of the current thread by tidying up connections to other stuff
+ */
+ public synchronized void dispose() {
+ if (!disposed) {
+ disposed = true;
+
+ // remove from parent thread group
+ threadGroup.remove(this);
+
+ // unlock all locked locks
+ unlockAll();
+
+ // clear all thread locals
+ clearThreadLocals();
+
+ // reset thread priority to initial if pooling
+ if (Options.THREADPOOL_ENABLED.load()) {
+ threadImpl.setPriority(initialPriority);
+ }
+
+ // mark thread as DEAD
+ beDead();
+
+ // unregister from runtime's ThreadService
+ getRuntime().getThreadService().unregisterThread(this);
+ }
}
public static RubyClass createThreadClass(Ruby runtime) {
@@ -459,6 +502,10 @@ private IRubyObject getSymbolKey(IRubyObject originalKey) {
return threadLocalVariables;
}
+ private void clearThreadLocals() {
+ threadLocalVariables = null;
+ }
+
public final Map<Object, IRubyObject> getContextVariables() {
return contextVariables;
}
View
21 src/org/jruby/internal/runtime/FutureThread.java
@@ -151,20 +151,29 @@ public void join(long millis) throws InterruptedException, ExecutionException {
}
/**
- * Jobs from the thread pool do not support setting priorities and always returns
- * current priority.
- *
+ * The current priority of the thread associated with this future.
+ *
* @return the current priority of the thread in which we this is running
*/
public int getPriority() {
- return 1;
+ if (nativeThread == null) {
+ return 1;
+ }
+
+ return nativeThread.getPriority();
}
/**
- * Jobs from the thread pool do not support setting priorities and always returns
- * current priority.
+ * Set the priority of the thread associated with this future.
+ *
+ * @param priority the new priority
*/
public void setPriority(int priority) {
+ if (nativeThread == null) {
+ return;
+ }
+
+ nativeThread.setPriority(priority);
}
public boolean isCurrent() {
View
8 src/org/jruby/internal/runtime/RubyRunnable.java
@@ -92,6 +92,7 @@ public void run() {
}
context.preRunThread(currentFrames);
+ rubyThread.beforeStart();
// uber-ThreadKill catcher, since it should always just mean "be dead"
try {
@@ -111,13 +112,8 @@ public void run() {
// Someone called exit!, so we need to kill the main thread
runtime.getThreadService().getMainThread().kill();
} finally {
- rubyThread.unlockAll();
runtime.getThreadService().setCritical(false);
- rubyThread.beDead();
-
- runtime.getThreadService().unregisterThread(rubyThread);
-
- ((RubyThreadGroup)rubyThread.group()).remove(rubyThread);
+ rubyThread.dispose();
// restore context classloader, in case we're using a thread pool
try {
View
21 test/test_thread.rb
@@ -45,9 +45,7 @@ def test_thread_local_variables
assert(Thread.current.key?(:x))
Thread.current["y"] = 2
assert(Thread.current.key?("y"))
- unless RUBY_VERSION =~ /1\.9/ # JRUBY-6485
- assert_equal([:x, :y], Thread.current.keys.sort {|x, y| x.to_s <=> y.to_s})
- end
+ assert_equal([:x, :y], Thread.current.keys.sort {|x, y| x.to_s <=> y.to_s} & [:x, :y])
assert_raises(TypeError) { Thread.current[Object.new] }
assert_raises(TypeError) { Thread.current[Object.new] = 1 }
assert_raises(ArgumentError) { Thread.current[1] }
@@ -156,12 +154,17 @@ def test_thread_subclass_zsuper
assert_equal(2, x.value)
end
- def test_dead_thread_priority
- x = Thread.new {}
- 1 while x.alive?
- x.priority = 5
- assert_equal(5, x.priority)
- end
+ # Because a Ruby thread may use a pooled thread, we will
+ # not preserve priorities set into dead threads. Because
+ # this is a meaningless feature, anyway, I remove it here
+ # and consider this behavior undefined. CON@20120306
+
+ # def test_dead_thread_priority
+ # x = Thread.new {}
+ # 1 while x.alive?
+ # x.priority = 5
+ # assert_equal(5, x.priority)
+ # end
def test_join_returns_thread
x = Thread.new {}
Please sign in to comment.
Something went wrong with that request. Please try again.