diff --git a/src/main/java/org/jboss/netty/channel/socket/nio/NioClientSocketPipelineSink.java b/src/main/java/org/jboss/netty/channel/socket/nio/NioClientSocketPipelineSink.java index da93ec03db..f69cb4d7e6 100644 --- a/src/main/java/org/jboss/netty/channel/socket/nio/NioClientSocketPipelineSink.java +++ b/src/main/java/org/jboss/netty/channel/socket/nio/NioClientSocketPipelineSink.java @@ -196,7 +196,7 @@ class NioClientSocketPipelineSink extends AbstractChannelSink { try { bossExecutor.execute( new IoWorkerRunnable(new ThreadRenamingRunnable( - this, "NewIO", "ClientBoss", String.valueOf(id), null))); + this, "NewIO", "ClientBoss", null, String.valueOf(id), null))); success = true; } finally { if (!success) { diff --git a/src/main/java/org/jboss/netty/channel/socket/nio/NioDatagramWorker.java b/src/main/java/org/jboss/netty/channel/socket/nio/NioDatagramWorker.java index e054769db8..68762403a8 100644 --- a/src/main/java/org/jboss/netty/channel/socket/nio/NioDatagramWorker.java +++ b/src/main/java/org/jboss/netty/channel/socket/nio/NioDatagramWorker.java @@ -169,7 +169,8 @@ class NioDatagramWorker implements Runnable { try { // Start the main selector loop. See run() for details. executor.execute(new ThreadRenamingRunnable( - this, "NewIO", "DatagramWorker", bossId + "-" + id, null)); + this, "NewIO", "DatagramWorker", + String.valueOf(bossId), String.valueOf(id), null)); success = true; } finally { if (!success) { diff --git a/src/main/java/org/jboss/netty/channel/socket/nio/NioServerSocketPipelineSink.java b/src/main/java/org/jboss/netty/channel/socket/nio/NioServerSocketPipelineSink.java index 743097ed30..4fb61f4e93 100644 --- a/src/main/java/org/jboss/netty/channel/socket/nio/NioServerSocketPipelineSink.java +++ b/src/main/java/org/jboss/netty/channel/socket/nio/NioServerSocketPipelineSink.java @@ -156,7 +156,7 @@ class NioServerSocketPipelineSink extends AbstractChannelSink { bossExecutor.execute( new IoWorkerRunnable(new ThreadRenamingRunnable( new Boss(channel), - "NewIO", "ServerBoss", String.valueOf(id), + "NewIO", "ServerBoss", null, String.valueOf(id), channel.toString()))); bossStarted = true; } catch (Throwable t) { diff --git a/src/main/java/org/jboss/netty/channel/socket/nio/NioWorker.java b/src/main/java/org/jboss/netty/channel/socket/nio/NioWorker.java index d6851a7d80..24b5ef83b3 100644 --- a/src/main/java/org/jboss/netty/channel/socket/nio/NioWorker.java +++ b/src/main/java/org/jboss/netty/channel/socket/nio/NioWorker.java @@ -112,7 +112,7 @@ class NioWorker implements Runnable { new IoWorkerRunnable(new ThreadRenamingRunnable( this, "NewIO", server? "ServerWorker" : "ClientWorker", - bossId + "-" + id, null))); + String.valueOf(bossId), String.valueOf(id), null))); success = true; } finally { if (!success) { diff --git a/src/main/java/org/jboss/netty/channel/socket/oio/OioClientSocketPipelineSink.java b/src/main/java/org/jboss/netty/channel/socket/oio/OioClientSocketPipelineSink.java index 2ab9c9f925..3cc76f223e 100644 --- a/src/main/java/org/jboss/netty/channel/socket/oio/OioClientSocketPipelineSink.java +++ b/src/main/java/org/jboss/netty/channel/socket/oio/OioClientSocketPipelineSink.java @@ -136,7 +136,7 @@ class OioClientSocketPipelineSink extends AbstractChannelSink { new ThreadRenamingRunnable( new OioWorker(channel), "OldIO", "ClientWorker", - id + "-" + channel.getId(), + String.valueOf(id), String.valueOf(channel.getId()), channel.toString()))); workerStarted = true; } catch (Throwable t) { diff --git a/src/main/java/org/jboss/netty/channel/socket/oio/OioDatagramPipelineSink.java b/src/main/java/org/jboss/netty/channel/socket/oio/OioDatagramPipelineSink.java index 4ef069f512..1d50647248 100644 --- a/src/main/java/org/jboss/netty/channel/socket/oio/OioDatagramPipelineSink.java +++ b/src/main/java/org/jboss/netty/channel/socket/oio/OioDatagramPipelineSink.java @@ -108,7 +108,7 @@ class OioDatagramPipelineSink extends AbstractChannelSink { new OioDatagramWorker(channel), "OldIO", "DatagramWorker", - id + "-" + channel.getId(), + String.valueOf(id), String.valueOf(channel.getId()), channel.toString()))); workerStarted = true; } catch (Throwable t) { @@ -154,13 +154,13 @@ class OioDatagramPipelineSink extends AbstractChannelSink { // Start the business. workerExecutor.execute(new IoWorkerRunnable(new ThreadRenamingRunnable( new OioDatagramWorker(channel), - service, category, id + "-" + channel.getId(), comment))); + service, category, String.valueOf(id), String.valueOf(channel.getId()), comment))); } else { // Worker started by bind() - just rename. Thread workerThread = channel.workerThread; if (workerThread != null) { ThreadRenamingRunnable.renameThread( - workerThread, service, category, id + "-" + channel.getId(), comment); + workerThread, service, category, String.valueOf(id), String.valueOf(channel.getId()), comment); } } diff --git a/src/main/java/org/jboss/netty/channel/socket/oio/OioDatagramWorker.java b/src/main/java/org/jboss/netty/channel/socket/oio/OioDatagramWorker.java index 334cc15a17..1e1d2d534f 100644 --- a/src/main/java/org/jboss/netty/channel/socket/oio/OioDatagramWorker.java +++ b/src/main/java/org/jboss/netty/channel/socket/oio/OioDatagramWorker.java @@ -177,7 +177,8 @@ class OioDatagramWorker implements Runnable { if (workerThread != null) { ThreadRenamingRunnable.renameThread( workerThread, "OldIO", "DatagramWorker", - ((OioDatagramChannelFactory) channel.getFactory()).id + "-" + channel.getId(), + String.valueOf(((OioDatagramChannelFactory) channel.getFactory()).id), + String.valueOf(channel.getId()), channel.toString()); } diff --git a/src/main/java/org/jboss/netty/channel/socket/oio/OioServerSocketPipelineSink.java b/src/main/java/org/jboss/netty/channel/socket/oio/OioServerSocketPipelineSink.java index 3723e446f9..f0ccc84813 100644 --- a/src/main/java/org/jboss/netty/channel/socket/oio/OioServerSocketPipelineSink.java +++ b/src/main/java/org/jboss/netty/channel/socket/oio/OioServerSocketPipelineSink.java @@ -151,8 +151,8 @@ class OioServerSocketPipelineSink extends AbstractChannelSink { new IoWorkerRunnable( new ThreadRenamingRunnable( new Boss(channel), - "OldIO", "ServerBoss", String.valueOf(id), - channel.toString()))); + "OldIO", "ServerBoss", null, + String.valueOf(id), channel.toString()))); bossStarted = true; } catch (Throwable t) { future.setFailure(t); @@ -218,7 +218,9 @@ class OioServerSocketPipelineSink extends AbstractChannelSink { new IoWorkerRunnable( new ThreadRenamingRunnable( new OioWorker(acceptedChannel), - "OldIO", "ServerWorker", id + "-" + acceptedChannel.getId(), + "OldIO", "ServerWorker", + String.valueOf(id), + String.valueOf(acceptedChannel.getId()), acceptedChannel.toString()))); } catch (Exception e) { logger.warn( diff --git a/src/main/java/org/jboss/netty/util/HashedWheelTimer.java b/src/main/java/org/jboss/netty/util/HashedWheelTimer.java index fb5f30051a..3926f554b4 100644 --- a/src/main/java/org/jboss/netty/util/HashedWheelTimer.java +++ b/src/main/java/org/jboss/netty/util/HashedWheelTimer.java @@ -211,7 +211,7 @@ public class HashedWheelTimer implements Timer { workerThread = threadFactory.newThread(new ThreadRenamingRunnable( worker, - "HashedWheelTimer", null, + "HashedWheelTimer", null, null, String.valueOf(id.incrementAndGet()), null)); // Misuse check diff --git a/src/main/java/org/jboss/netty/util/ThreadNameDeterminer.java b/src/main/java/org/jboss/netty/util/ThreadNameDeterminer.java index 3aa3d73610..4baa2c118e 100644 --- a/src/main/java/org/jboss/netty/util/ThreadNameDeterminer.java +++ b/src/main/java/org/jboss/netty/util/ThreadNameDeterminer.java @@ -30,13 +30,13 @@ public interface ThreadNameDeterminer { */ ThreadNameDeterminer PROPOSED = new ThreadNameDeterminer() { public String determineThreadName(String current, String service, - String category, String id, String comment) throws Exception { + String category, String parentId, String id, String comment) throws Exception { String newName = - (format("", service, " ") + - format("", category, " ") + - format("#", id, " ") + - format("(", comment, ")")).trim(); + (format("", " ", service) + + format("", " ", category) + + format("#", " ", parentId, id) + + format("(", ")", comment)).trim(); if (newName.length() == 0) { return null; } else { @@ -44,12 +44,22 @@ public interface ThreadNameDeterminer { } } - private String format(String prefix, String s, String postfix) { - if (s.length() == 0) { - return ""; - } else { - return prefix + s + postfix; + private String format(String prefix, String postfix, String... components) { + StringBuilder buf = new StringBuilder(); + for (String c: components) { + if (c.length() == 0) { + continue; + } + buf.append(c); + buf.append(':'); } + + if (buf.length() == 0) { + return ""; + } + + buf.setLength(buf.length() - 1); // Remove trailing ':' + return prefix + buf + postfix; } }; @@ -59,7 +69,7 @@ public interface ThreadNameDeterminer { */ ThreadNameDeterminer CURRENT = new ThreadNameDeterminer() { public String determineThreadName(String current, String service, - String category, String id, String comment) throws Exception { + String category, String parentId, String id, String comment) throws Exception { return null; } }; @@ -68,9 +78,10 @@ public interface ThreadNameDeterminer { * Overrides the thread name proposed by {@link ThreadRenamingRunnable}. * * @param current the current thread name - * @param service the service name (e.g. "New I/O" or "Old I/O") - * @param category the category name (e.g. "server boss" or "client worker") - * @param id the thread ID (e.g. "1" or "1-3") + * @param service the service name (e.g. "NewIO" or "OldIO") + * @param category the category name (e.g. "ServerBoss" or "ClientWorker") + * @param parentId the parent thread ID (e.g. "1") + * @param id the thread ID (e.g. "3") * @param comment the optional comment which might help debugging * * @return the actual new thread name. @@ -79,5 +90,5 @@ public interface ThreadNameDeterminer { */ String determineThreadName( String current, - String service, String category, String id, String comment) throws Exception; + String service, String category, String parentId, String id, String comment) throws Exception; } diff --git a/src/main/java/org/jboss/netty/util/ThreadRenamingRunnable.java b/src/main/java/org/jboss/netty/util/ThreadRenamingRunnable.java index 471c47d443..fc7c40fe07 100644 --- a/src/main/java/org/jboss/netty/util/ThreadRenamingRunnable.java +++ b/src/main/java/org/jboss/netty/util/ThreadRenamingRunnable.java @@ -42,8 +42,7 @@ public class ThreadRenamingRunnable implements Runnable { private static final Pattern SERVICE_PATTERN = Pattern.compile("[a-zA-Z0-9]*"); private static final Pattern CATEGORY_PATTERN = SERVICE_PATTERN; - private static final Pattern ID_PATTERN = Pattern.compile("^[-_a-zA-Z0-9]*$"); - + private static final Pattern ID_PATTERN = SERVICE_PATTERN; private static volatile ThreadNameDeterminer threadNameDeterminer = ThreadNameDeterminer.PROPOSED; @@ -78,16 +77,17 @@ public class ThreadRenamingRunnable implements Runnable { * * @return {@code true} if and only if the thread was renamed */ - public static boolean renameThread(Thread thread, String service, String category, String id, String comment) { + public static boolean renameThread(Thread thread, String service, String category, String parentId, String id, String comment) { if (thread == null) { throw new NullPointerException("thread"); } - validateNameComponents(service, category, id); + validateNameComponents(service, category, parentId, id); // Normalize the parameters. service = service != null? service : ""; category = category != null? category : ""; + parentId = parentId != null? parentId : ""; id = id != null? id : ""; comment = comment != null? comment : ""; @@ -96,7 +96,7 @@ public class ThreadRenamingRunnable implements Runnable { String newThreadName = null; try { newThreadName = getThreadNameDeterminer().determineThreadName( - oldThreadName, service, category, id, comment); + oldThreadName, service, category, parentId, id, comment); } catch (Throwable t) { logger.warn("Failed to determine the thread name", t); } @@ -121,7 +121,7 @@ public class ThreadRenamingRunnable implements Runnable { return renamed; } - private static void validateNameComponents(String service, String category, String id) { + private static void validateNameComponents(String service, String category, String parentId, String id) { if (service != null && !SERVICE_PATTERN.matcher(service).matches()) { throw new IllegalArgumentException( "service: " + service + @@ -134,6 +134,12 @@ public class ThreadRenamingRunnable implements Runnable { " (expected: " + CATEGORY_PATTERN.pattern() + ')'); } + if (parentId != null && !ID_PATTERN.matcher(parentId).matches()) { + throw new IllegalArgumentException( + "parentId: " + parentId + + " (expected: " + ID_PATTERN.pattern() + ')'); + } + if (id != null && !ID_PATTERN.matcher(id).matches()) { throw new IllegalArgumentException( "id: " + id + @@ -144,6 +150,7 @@ public class ThreadRenamingRunnable implements Runnable { private final Runnable runnable; private final String service; private final String category; + private final String parentId; private final String id; private final String comment; @@ -154,15 +161,16 @@ public class ThreadRenamingRunnable implements Runnable { */ public ThreadRenamingRunnable( Runnable runnable, - String service, String category, String id, String comment) { + String service, String category, String parentId, String id, String comment) { if (runnable == null) { throw new NullPointerException("runnable"); } - validateNameComponents(service, category, id); + validateNameComponents(service, category, parentId, id); this.runnable = runnable; this.service = service; this.category = category; + this.parentId = parentId; this.id = id; this.comment = comment; } @@ -173,7 +181,7 @@ public class ThreadRenamingRunnable implements Runnable { // Change the thread name before starting the actual runnable. final boolean renamed = renameThread( - Thread.currentThread(), service, category, id, comment); + Thread.currentThread(), service, category, parentId, id, comment); // Run the actual runnable and revert the name back when it ends. try { diff --git a/src/test/java/org/jboss/netty/util/ThreadRenamingRunnableTest.java b/src/test/java/org/jboss/netty/util/ThreadRenamingRunnableTest.java index 40386f9bf4..28cba88126 100644 --- a/src/test/java/org/jboss/netty/util/ThreadRenamingRunnableTest.java +++ b/src/test/java/org/jboss/netty/util/ThreadRenamingRunnableTest.java @@ -34,7 +34,7 @@ public class ThreadRenamingRunnableTest { @Test(expected = NullPointerException.class) public void shouldNotAllowNullRunnable() throws Exception { - new ThreadRenamingRunnable(null, "a", "b", "c", "d"); + new ThreadRenamingRunnable(null, "a", "b", "c", "d", "e"); } @Test @@ -46,7 +46,7 @@ public class ThreadRenamingRunnableTest { public void run() { assertEquals(oldThreadName, Thread.currentThread().getName()); } - }, null, null, null, null)); + }, null, null, null, null, null)); assertEquals(oldThreadName, Thread.currentThread().getName()); } @@ -60,7 +60,7 @@ public class ThreadRenamingRunnableTest { public void run() { assertEquals(oldThreadName, Thread.currentThread().getName()); } - }, "", "", "", "")); + }, "", "", "", "", "")); assertEquals(oldThreadName, Thread.currentThread().getName()); } @@ -72,10 +72,10 @@ public class ThreadRenamingRunnableTest { e.execute(new ThreadRenamingRunnable( new Runnable() { public void run() { - assertEquals("a b #c (d)", Thread.currentThread().getName()); + assertEquals("a b #c:d (e)", Thread.currentThread().getName()); assertFalse(oldThreadName.equals(Thread.currentThread().getName())); } - }, "a", "b", "c", "d")); + }, "a", "b", "c", "d", "e")); assertEquals(oldThreadName, Thread.currentThread().getName()); } @@ -107,7 +107,7 @@ public class ThreadRenamingRunnableTest { public void run() { assertEquals(oldThreadName, Thread.currentThread().getName()); } - }, "a", "b", "c", "d")); + }, "a", "b", "c", "d", "e")); } finally { System.setSecurityManager(null); assertEquals(oldThreadName, Thread.currentThread().getName());