[Core] Use meaningful thread names #1623
Conversation
|
|
||
| CucumberThreadFactory() { | ||
| SecurityManager s = System.getSecurityManager(); | ||
| this.group = s != null ? s.getThreadGroup() : Thread.currentThread().getThreadGroup(); |
There was a problem hiding this comment.
I don't think it's required nor desirable to manually set the thread group. The ThreadGroup for the workers should be the same as the ThreadGroup of Runtime.run. Currently it is the ThreadGroup of Runtime.Builder.build which need not be the same. Using a different constructor will automatically use the correct ThreadGroup.
|
|
||
| @Override | ||
| public Thread newThread(Runnable r) { | ||
| Thread t = new Thread(this.group, r, "cucumber-runner-thread-" + this.threadNumber.getAndIncrement(), 0L); |
There was a problem hiding this comment.
Using the Thread(String name) constructor should be the safest, least confusing option here.
| public Thread newThread(Runnable r) { | ||
| Thread t = new Thread(this.group, r, "cucumber-runner-thread-" + this.threadNumber.getAndIncrement(), 0L); | ||
| if (t.isDaemon()) { | ||
| t.setDaemon(false); |
There was a problem hiding this comment.
Is there a reason to override the inherited settings?
| } | ||
|
|
||
| if (t.getPriority() != 5) { | ||
| t.setPriority(5); |
There was a problem hiding this comment.
Is there a reason to override the inherited settings?
|
I just wanted to mirror the behaviour of Executors.defaultThreadFactory() - that's the one which was used effectively before my change - as closely as possible. But I'm also fine with removing some of the none strictly necessary stuff (groups, priority) |
|
Actually, we should probably still keep the |
|
Maybe |
|
I'd say |
Ah that makes sense. But it's all behavior we don't quite need. So it's better to keep the code simple. |
mpkorstanje
changed the title
|
Cheers! |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
When running with --threads, give the the pool threads some meaningful names
Details
Before it was using the standard threadfactory, which was naming threads like 'pool-x-thread-y'
Now it is 'cucumber-runner-thread-x'
Motivation and Context
It's better for logging to see which threads belong to cucumber and which are unrelated background threads