On Mon, 28 Nov 2022 15:47:48 GMT, Jaikiran Pai <[email protected]> wrote:
> `#1` is where I am having more and more questions. But before I get to those, > I was looking at the test log run that failed and I couldn't find any crucial > debug logs that our HttpClient generates. That would have given some hint on > whether or not the selector manager thread shutdown was initiated. Looks like > the `SpecialHeadersTest` doesn't set the `jdk.internal.httpclient.debug`. > That also means that our logic in the `beforeMethod` to stop on first failure > ended up being a no-op. Perhaps we should add that system property and wait > for some runs to fail to capture the necessary details? What I could see from the log - and that was enough to understand the issue, is that the test observed "alive" being false, so decided that it didn't need to wait, then immediately after saw "alive" being true - and handled that is if alive was observed after the timeout had expired - when it had waited 0ms for the thread to shutdown. This is what this fix is fixing. We must wait until the thread has started, and has shut down, or the timeout has expired. We should stop waiting after the thread has exited, but keep on waiting until it has started & shutdown (unless Thread::start itself failed). That latter is not a condition I have ever observed - so the catch around selmgr::start is 'just' to keep things consistent. ------------- PR: https://git.openjdk.org/jdk/pull/11294
