Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability for timeout to print full thread stack dump #463

Open
toddlipcon opened this issue Jul 16, 2012 · 17 comments
Open

Add ability for timeout to print full thread stack dump #463

toddlipcon opened this issue Jul 16, 2012 · 17 comments

Comments

@toddlipcon
Copy link

When using JUnit to run integration-style tests which involve several threads, or to test multi-threaded components, a deadlock or livelock bug can often result in a test timeout. The current behavior only provides the stack trace of the main test thread, which is not always sufficient to understand the true cause of the deadlock (the main thread may just be blocked on another worker thread).

I'd like to add the ability to dump all of the thread stacks to stderr when a timeout is triggered. Does this seem like a reasonable default behavior, or something best enabled via some kind of property/configuration?

@kcooney
Copy link
Member

kcooney commented Jul 17, 2012

It would be quite easy to write a custom Rule to do this; org.junit.rules.Timeout should be quite close to what you want.

I wonder if we should change Timeout to throw a typed exception on timeout. If we did, you could write a custom rule that caught that exception and did some extra handling (print all of the tread stacks, throw a new exception that contained all of the stack dumps, etc)

@toddlipcon
Copy link
Author

I am a bit of a noob when it comes to advanced JUnit. Is there a way to apply a Rule to all tests in a large maven project, without manually annotating every last test with @rule? Seems a bit repetitive to add to all ~4000 test cases, and hard to remind people to add it to new cases as they add them.

@kcooney
Copy link
Member

kcooney commented Jul 17, 2012

I assume you want every test method in your project to have a specified timeout. If so, I don't know of a way to add a timeout to all the test methods in your project.

You could have the test classes that need a timeout inherit from a base class that has the rule declared. I believe you could also specify a timeout for an entire suite by adding a @ClassRule to the outer suite.

You could, of course, write a custom runner that applied a timeout to all of the tests that used the runner, but you would have to update all your test classes to use the custom runner, which would be just as much work as adding a Rule but would be less flexible.

@dsaff
Copy link
Member

dsaff commented Jul 17, 2012

@toddlipcon, how is your timeout being set currently? Are you running through ant or maven, which supply their own timeouts?

@toddlipcon
Copy link
Author

We're setting timeout on each test with a @timeout annotation. But, this is a nuisance too -- people often forget to add timeouts, and it's much harder to diagnose those failures. So a global rule (and timeout) setting would be helpful.

To answer the other question, we currently run the tests through maven using surefire (the project in question is Apache Hadoop).

@dsaff
Copy link
Member

dsaff commented Jul 17, 2012

Let's split the bugs: one is printing out more than one thread stacktrace when a test times out (ideally, I take it, triggered by the @timeout annotation).

Two is a global setting that affects all methods by adding a timeout.

Both are interesting.

Which should we track in this feature request, and which should we move to a new request? Thanks.

@toddlipcon
Copy link
Author

Let's keep this one about the multi-thread stack dump, and separately talk about the global rule application. Sound good?

@dsaff
Copy link
Member

dsaff commented Jul 17, 2012

Works for me.

One concern I have about the multi-thread dump is that there may be many threads in flight, only some of which are interesting to the test. Would it make sense to ask users to somehow tag or register the threads they would eventually need stacktraces for?

@toddlipcon
Copy link
Author

I'd rather err on the side of giving all the threads in the dump (same as a SIGQUIT). Perhaps have some way to allow users to tag threads as un-interesting, but seems like when you're debugging a failure, it's not too bad to manually ignore the threads you don't care about. Whereas by default you'd like to see all the threads you spawned.

@Tibor17
Copy link
Contributor

Tibor17 commented Jul 17, 2012

What about introducing three types of runners dumping
current thread,
all threads,
threads filtered by name of thread using hamcrest Matcher with String.

Registering threads in a global instance may introduce memory leaks and/or app hangs.

@toddlipcon
Copy link
Author

Ah, the hamcrest matcher is quite a clever idea. I would have the matcher actually take the thread itself, instead of just the thread's name (eg a user might already have convenient marker interfaces attached to their threads)

@Tibor17
Copy link
Contributor

Tibor17 commented Jul 17, 2012

it seems the solution already exists
http://www.attivio.com/blog/56-java-development/379-enhancing-junit-with-default-timeouts-and-stack-traces.html
@toddlipcon Is this what you are looking for?

@toddlipcon
Copy link
Author

Yep, that's basically what I'm looking for without having the extra complexity of getting a different version of junit on the maven classpath to build, etc. I think these should be common enough use cases that it belongs in "stock" junit without every project having to customize it. Maybe that's against the JUnit philosophy though.

@Tibor17
Copy link
Contributor

Tibor17 commented Jul 17, 2012

never mind, but look at the code.
i think it matches, right?

@Tibor17
Copy link
Contributor

Tibor17 commented Jul 17, 2012

If it would solve your problem and @dsaff accepts a new feature in pull request, then you may have it in some of the next junit release.

I underastand selecting threads just as another feature for you, or did i miss something?

@dsaff
Copy link
Member

dsaff commented Jul 18, 2012

I think the basic idea makes sense.

Someone should try a first-cut implementation, and see whether there's a way to make the information available without also cluttering the main output.

@dsaff
Copy link
Member

dsaff commented Nov 15, 2013

#742 is a clear first step in this direction.

reinholdfuereder added a commit to reinholdfuereder/junit that referenced this issue Nov 22, 2013
junit-team#463: Add ability for timeout to print full thread stack dump
- Naive attempt that (a) optionally adds information from ThreadMXBean#findMonitorDeadlockedThreads and (b) always adds full thread dump to exception message
reinholdfuereder added a commit to reinholdfuereder/junit that referenced this issue Nov 22, 2013
junit-team#463 Add ability for timeout to print full thread stack dump
- Another attempt that allows to "install" an additional custom timeout handler that can optionally also add a further exception to the test timeout exception
reinholdfuereder added a commit to reinholdfuereder/junit that referenced this issue Nov 22, 2013
junit-team#463 Add ability for timeout to print full thread stack dump
- Another attempt that allows to "install" an additional custom timeout handler that can optionally also add a further exception to the test timeout exception
- Fix: remove outdated TODO comment
reinholdfuereder added a commit to reinholdfuereder/junit that referenced this issue Nov 22, 2013
junit-team#463 Add ability for timeout to print full thread stack dump
- Extend the attempt that allows to "install" an additional custom timeout handler (that can optionally also add a further exception to the test timeout exception) by a globally set custom timeout handler based on a system property: note this fails in Ant context; and deadlocked threads of self tests are surviving; so this is by no means finished
reinholdfuereder added a commit to reinholdfuereder/junit that referenced this issue Nov 23, 2013
junit-team#463 Add ability for timeout to print full thread stack dump
- Ref tests and where the test thread is interrupted to prevent surviving threads (as it is the case for other older tests!), reduce duplication in test code
windie added a commit to windie/netty that referenced this issue Jan 24, 2016
Motivation:

See netty#3172

Modifications:

Copy TimedOutTestsListener from Apache Hadoop(https://github.com/apache/hadoop/blob/d1c6accb6f87b08975175580e15f1ff1fe29ab04/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/TimedOutTestsListener.java) into the "common" project and add `output.flush()`. If not calling `flush`, the output may be printed to the console.

In addition, also update the `pom.xml`s to set up the timeout listener.

This is a workaround before junit adds such listener (junit-team/junit4#463).

Result:

If a test is set the timeout paramter using junit's `@Test(timeout = ...)` and the timeout is triggered, a full stack trace dump will be outputted and also output the deadlocks if any.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants