Skip to content

Commit

Permalink
Forbid non-named threads
Browse files Browse the repository at this point in the history
This commit forbids the use of Thread constructors that do not take a
name. In general nameless threads are more difficult to understand their
purpose when debugging. Note that this is only added to production
signatures. Tests are not forbidden here so as not to be pedantic (or
require a larger change since many tests create anonymous threads).

relates elastic#101628
  • Loading branch information
rjernst committed Oct 31, 2023
1 parent 73eb3ec commit d87934d
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,7 @@ org.elasticsearch.cluster.ClusterState#compatibilityVersions()
org.elasticsearch.cluster.ClusterFeatures#nodeFeatures()
@defaultMessage ClusterFeatures#clusterHasFeature is for internal use only. Use FeatureService#clusterHasFeature to determine if a feature is present on the cluster.
org.elasticsearch.cluster.ClusterFeatures#clusterHasFeature(org.elasticsearch.features.NodeFeature)

@defaultMessage Use a Thread constructor with a name, anonymous threads are more difficult to debug
java.lang.Thread#<init>(java.lang.Runnable)
java.lang.Thread#<init>(java.lang.ThreadGroup, java.lang.Runnable)
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ static Thread createShutdownHook(Terminal terminal, Closeable closeable) {
e.printStackTrace(terminal.getErrorWriter());
}
terminal.flush(); // make sure to flush whatever the close or error might have written
});
}, "elasticsearch-cli-shutdown");

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ public static void maybeDieOnAnotherThread(final Throwable throwable) {
final String formatted = ExceptionsHelper.formatStackTrace(Thread.currentThread().getStackTrace());
logger.error("fatal error {}: {}\n{}", error.getClass().getCanonicalName(), error.getMessage(), formatted);
} finally {
new Thread(() -> { throw error; }).start();
new Thread(() -> { throw error; }, "elasticsearch-error-rethrower").start();
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ private static void initPhase2(Bootstrap bootstrap) throws IOException {
// initialize probes before the security manager is installed
initializeProbes();

Runtime.getRuntime().addShutdownHook(new Thread(Elasticsearch::shutdown));
Runtime.getRuntime().addShutdownHook(new Thread(Elasticsearch::shutdown, "elasticsearch-shutdown"));

// look for jar hell
final Logger logger = LogManager.getLogger(JarHell.class);
Expand Down Expand Up @@ -376,7 +376,7 @@ private static void startCliMonitorThread(InputStream stdin) {
Bootstrap.exit(1);
}
}
}).start();
}, "elasticsearch-cli-monitor-thread").start();
}

/**
Expand Down

0 comments on commit d87934d

Please sign in to comment.