jruby · Dec 4, 2015 · Dec 4, 2015 · Dec 4, 2015 · Dec 4, 2015
Showing with 150 additions and 38 deletions.

+77 −17 core/src/main/java/org/jruby/RubyThread.java

+73 −21 test/jruby/test_thread.rb
diff --git a/core/src/main/java/org/jruby/RubyThread.java b/core/src/main/java/org/jruby/RubyThread.java
@@ -68,6 +68,7 @@
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicIntegerFieldUpdater;
+import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.locks.Lock;
 
@@ -105,8 +106,6 @@ public class RubyThread extends RubyObject implements ExecutionContext {
 
     private static final Logger LOG = LoggerFactory.getLogger("RubyThread");
 
-    private volatile IRubyObject threadName;
-
     /** The thread-like think that is actually executing */
     private volatile ThreadLike threadImpl;
 
@@ -125,6 +124,9 @@ public class RubyThread extends RubyObject implements ExecutionContext {
     /** The final value resulting from the thread's execution */
     private volatile IRubyObject finalResult;
 
+    private volatile IRubyObject threadName;
+    private String file; private int line; // Thread.new location (for inspect)
+
     /**
      * The exception currently being raised out of the thread. We reference
      * it here to continue propagating it while handling thread shutdown
@@ -556,7 +558,9 @@ private IRubyObject startThread(ThreadContext context, Runnable runnable) throws
         try {
             Thread thread = new Thread(runnable);
             thread.setDaemon(true);
-            setThreadName(runtime, thread, context.getFile(), context.getLine());
+            this.file = context.getFile();
+            this.line = context.getLine();
+            setThreadName(runtime, thread, file, line, true);
             threadImpl = new NativeThread(this, thread);
 
             addToCorrectThreadGroup(context);
@@ -583,15 +587,62 @@ private IRubyObject startThread(ThreadContext context, Runnable runnable) throws
         }
     }
 
-    private static void setThreadName(final Ruby runtime, final Thread thread,
-        final String file, final int line) {
-        final StringBuilder name = new StringBuilder(24);
-        name.append("Ruby-").append(runtime.getRuntimeNumber());
-        name.append('-').append(thread.getName());
-        if ( file != null ) {
-            name.append(':').append(' ').append(file).append(':').append(line + 1);
+    private static final String RUBY_THREAD_PREFIX = "Ruby-";
+
+    private void setThreadName(final Ruby runtime, final Thread thread,
+        final String file, final int line, final boolean newThread) {
+        // "Ruby-0-Thread-16: (irb):21"
+        // "Ruby-0-Thread-17@worker#1: (irb):21"
+        final String newName;
+        final String setName = getNameOrNull();
+        final String currentName = thread.getName();
+        if ( currentName != null && currentName.startsWith(RUBY_THREAD_PREFIX) ) {
+            final int i = currentName.indexOf('@'); // Thread#name separator
+            if ( i == -1 ) { // name not set yet: "Ruby-0-Thread-42: FILE:LINE"
+                int end = currentName.indexOf(':');
+                if ( end == -1 ) end = currentName.length();
+                final String prefix = currentName.substring(0, end);
+                newName = currentName.replace(prefix, prefix + '@' + setName);
+
+            }
+            else { // name previously set: "Ruby-0-Thread-42@foo: FILE:LINE"
+                final String prefix = currentName.substring(0, i); // Ruby-0-Thread-42
+                int end = currentName.indexOf(':', i);
+                if ( end == -1 ) end = currentName.length();
+                final String prefixWithName = currentName.substring(0, end); // Ruby-0-Thread-42@foo:
+                newName = currentName.replace(prefixWithName, setName == null ? prefix : (prefix + '@' + setName));
+            }
+        }
+        else if ( newThread ) {
+            final StringBuilder name = new StringBuilder(24);
+            name.append(RUBY_THREAD_PREFIX).append(runtime.getRuntimeNumber());
+            name.append('-').append("Thread-").append(incAndGetThreadCount(runtime));
+            if ( setName != null ) name.append('@').append(setName);
+            if ( file != null ) { // in JIT we seem to get "" as file and line 0
+                name.append(':').append(' ').append(file).append(':').append(line + 1);
+            }
+            newName = name.toString();
         }
-        thread.setName(name.toString());
+        else return; // not a new-thread that and does not match out Ruby- prefix
+        // ... very likely user-code set the java thread name - thus do not mess!
+        try { thread.setName(newName); }
+        catch (SecurityException ignore) { } // current thread can not modify
+    }
+
+    // TODO likely makes sense to have a counter or the Ruby class directly (could be included with JMX)
+    private static final WeakHashMap<Ruby, AtomicLong> threadCount = new WeakHashMap<Ruby, AtomicLong>(4);
+
+    private static long incAndGetThreadCount(final Ruby runtime) {
+        AtomicLong counter = threadCount.get(runtime);
+        if ( counter == null ) {
+            synchronized (runtime) {
+                counter = threadCount.get(runtime);
+                if ( counter == null ) {
+                    threadCount.put(runtime, counter = new AtomicLong(0));
+                }
+            }
+        }
+        return counter.incrementAndGet();
     }
 
     private static RubyThread startThread(final IRubyObject recv, final IRubyObject[] args, boolean callInit, Block block) {
@@ -703,14 +754,21 @@ public IRubyObject pending_interrupt_p(ThreadContext context, IRubyObject[] args
 
     @JRubyMethod(name = "name=", required = 1)
     public IRubyObject setName(IRubyObject name) {
-        return this.threadName = name;
+        this.threadName = name;
+        setThreadName(getRuntime(), getNativeThread(), null, -1, false);
+        return name;
     }
 
     @JRubyMethod(name = "name")
     public IRubyObject getName() {
         return this.threadName;
     }
 
+    private String getNameOrNull() {
+        final IRubyObject name = getName();
+        return ( name == null || name.isNil() ) ? null : name.asJavaString();
+    }
+
     private boolean pendingInterruptInclude(IRubyObject err) {
         Iterator<IRubyObject> iterator = pendingInterruptQueue.iterator();
         while (iterator.hasNext()) {
@@ -1022,14 +1080,16 @@ void setThreadGroup(RubyThreadGroup rubyThreadGroup) {
     @Override
     public synchronized IRubyObject inspect() {
         // FIXME: There's some code duplication here with RubyObject#inspect
-        StringBuilder part = new StringBuilder();
+        StringBuilder part = new StringBuilder(32);
         String cname = getMetaClass().getRealClass().getName();
         part.append("#<").append(cname).append(':');
         part.append(identityString());
-        final IRubyObject name = getName(); // thread.name
-        if (name != null && ! name.isNil()) {
-            part.append('@');
-            part.append(name.asJavaString());
+        final String name = getNameOrNull(); // thread.name
+        if ( name != null ) {
+            part.append('@').append(name);
+        }
+        if ( file != null && line > 0 ) {
+            part.append('@').append(file).append(':').append(line);
         }
         part.append(' ');
         part.append(status.toString().toLowerCase());

diff --git a/test/jruby/test_thread.rb b/test/jruby/test_thread.rb
@@ -58,7 +58,7 @@ def test_status
     v = t.value
     assert_equal("run", v)
     assert_equal(false, t.status)
-    
+
     # check that "run", sleep", and "dead" appear in inspected output
     q = Queue.new
     ready = false
@@ -71,11 +71,6 @@ def test_status
     assert(t.inspect["dead"])
   end
 
-  def test_inspect
-    t = Thread.new {}.join
-    assert_match /#<Thread:0x[0-9a-z]+ \w+>/, t.inspect
-  end
-
   def thread_foo()
     raise "hello"
   end
@@ -118,7 +113,7 @@ def test_raise
       e = error
     end
     assert(e.kind_of?(RuntimeError))
-    
+
     # test raising in a sleeping thread
     e = 1
     set = false
@@ -129,7 +124,7 @@ def test_raise
       end
       t.raise("Die")
     rescue; end
-    
+
     assert_equal(2, e)
     assert_raise(RuntimeError) { t.value }
   end
@@ -139,13 +134,13 @@ def test_thread_value
     assert_equal(2, Thread.new { 2 }.value)
     assert_raise(RuntimeError) { Thread.new { raise "foo" }.value }
   end
-  
+
   class MyThread < Thread
     def initialize
       super do; 1; end
     end
   end
-  
+
   def test_thread_subclass_zsuper
     x = MyThread.new
     x.join
@@ -154,24 +149,24 @@ def test_thread_subclass_zsuper
     x.join
     assert_equal(2, x.value)
   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 {}
     assert_nothing_raised { x.join.to_s }
   end
-  
+
   def test_abort_on_exception_does_not_blow_up
     # CON: I had an issue where annotated methods weren't binding right
     # where there was both a static and instance method of the same name.
@@ -221,7 +216,7 @@ def test_socket_accept_can_be_interrupted
     t.raise ex
     assert_raises(Exception) { t.join }
   end
-  
+
   # JRUBY-2315
   def test_exit_from_within_thread
     begin
@@ -235,7 +230,7 @@ def test_exit_from_within_thread
         sleep 0.5
         Kernel.exit(1)
       end
- 
+
       a.join
       fail
       b.join
@@ -251,7 +246,7 @@ def test_exit_from_within_thread
   def call_to_s(a)
     a.to_s
   end
-  
+
   # JRUBY-2477 - polymorphic calls are not thread-safe
   def test_poly_calls_thread_safe
     # Note this isn't a perfect test, but it's not possible to test perfectly
@@ -260,9 +255,9 @@ def test_poly_calls_thread_safe
     threads = []
     sym = :foo
     str = "foo"
-    
+
     20.times {|i| threads << Thread.new { 10_000.times { call_to_s(sym); call_to_s(str) }; results[i] = true }}
-    
+
     threads.pop.join until threads.empty?
     assert_equal [true] * 20, results
   end
@@ -288,7 +283,7 @@ def test_new_thread_in_list
       t.join
     end
   end
-  
+
   # JRUBY-3568: thread group is inherited from parent
   def test_inherits_thread_group
     tg = ThreadGroup.new
@@ -321,7 +316,6 @@ def test_wakeup_wakes_sleeping_thread
 
   # JRUBY-5290
   def test_default_priority
-    require 'java'
     t = Thread.new { sleep 1 while true }
     assert_equal 0, t.priority
     t.exit
@@ -347,4 +341,62 @@ def test_sleep_wakeup_interlacing
     t.join
     assert_equal(10000, ret.size)
   end
+
+  def test_inspect_and_to_s
+    t = Thread.new {}.join
+    assert_match /#<Thread:0x[0-9a-z]+>/, t.to_s
+    # TODO we do not have file/line right :
+    # MRI: #<Thread:0x000000014b0e28@test/jruby/test_thread.rb:346 dead>
+    #assert_match /#<Thread:0x[0-9a-z]+@test\/jruby\/test_thread\.rb\:346 \w+>/, t.inspect
+    assert_match /#<Thread:0x[0-9a-z]+(@.*\.rb\:\d+)? \w+>/, t.inspect
+
+    assert_nil t.name
+
+    t = Thread.new {}.join
+    t.name = 'universal'
+    assert_match /#<Thread:0x[0-9a-z]+>/, t.to_s
+    assert_match /#<Thread:0x[0-9a-z]+@universal(@.*\.rb\:\d+)? \w+>/, t.inspect
+  end
+
+  def test_thread_name
+    Thread.new do
+      assert_match /\#\<Thread\:0x\h+(@[\w\/\._]+\:\d+)?\srun\>/, Thread.current.inspect
+      # TODO? currently in JIT file comes as "" and line as 0
+      assert_match /Ruby\-\d+\-Thread\-\d+\:\s(.*\.rb)?\:\d+/, native_thread_name(Thread.current) if defined? JRUBY_VERSION
+    end.join
+
+    Thread.new do
+      Thread.current.name = 'foo'
+      assert_match /\#\<Thread\:0x\h+@foo(@[\w\/\._]+\:\d+)?\srun\>/, Thread.current.inspect
+      assert_match /Ruby\-\d+\-Thread\-\d+\@foo:\s(.*\.rb)?\:\d+/, native_thread_name(Thread.current) if defined? JRUBY_VERSION
+
+      Thread.current.name = 'bar'
+      assert_match /\#\<Thread\:0x\h+@bar(@[\w\/\._]+\:\d+)?\srun\>/, Thread.current.inspect
+      assert_match /Ruby\-\d+\-Thread\-\d+\@bar:\s(.*\.rb)?\:\d+/, native_thread_name(Thread.current) if defined? JRUBY_VERSION
+
+      Thread.current.name = nil
+      assert_match /\#\<Thread\:0x\h+(@[\w\/\._]+\:\d+)?\srun\>/, Thread.current.inspect
+      assert_match /Ruby\-\d+\-Thread\-\d+\:\s(.*\.rb)?\:\d+/, native_thread_name(Thread.current) if defined? JRUBY_VERSION
+    end.join
+
+
+    Thread.new do
+      Thread.current.to_java.native_thread.name = 'user-set-native-thread-name'
+      Thread.current.name = 'foo'
+
+      assert Thread.current.inspect.index('@foo')
+      assert_equal 'user-set-native-thread-name', native_thread_name(Thread.current) if defined? JRUBY_VERSION
+
+      Thread.current.name = nil
+      assert ! Thread.current.inspect.index('@foo')
+      assert_equal 'user-set-native-thread-name', native_thread_name(Thread.current) if defined? JRUBY_VERSION
+    end.join
+  end
+
+  private
+
+  def native_thread_name(thread)
+    thread.to_java.native_thread.name
+  end
+
 end