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

QueuedThreadPool: Stopped without executing or closing null #7650

Closed
dzoech opened this issue Feb 24, 2022 · 9 comments · Fixed by #9325
Closed

QueuedThreadPool: Stopped without executing or closing null #7650

dzoech opened this issue Feb 24, 2022 · 9 comments · Fixed by #9325
Labels
Bug For general bugs on Jetty side

Comments

@dzoech
Copy link
Contributor

dzoech commented Feb 24, 2022

Jetty version(s)
9.4.44.v20210927

Java version/vendor (use: java -version)
Java 17
we use gcr.io/distroless/java17-debian11:nonroot

OS type/version
Debian 11

Description
We use Spring Boot 2.6.2 with Jetty.

In our logs the following log appears:

WARN [SpringApplicationShutdownHook] QueuedThreadPool: Stopped without executing or closing null

This seems to be the code that writes that warning log:
https://github.com/eclipse/jetty.project/blob/jetty-9.4.44.v20210927/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java#L299

My understand is that either the BlockingQueue<Runnable> _jobs contains objects that are null, or there is a race condition so that !_jobs.isEmpty() is true but then _jobs.poll() returns null because it is now empty.

I'm not sure if this issue could be related to Spring so I'd like to mention that we implemented a shutdown hook waiting for an ExecutorService:

@Configuration
public class MyConfig {
    ....
    @Bean(destroyMethod = "onShutdown")
    MyWorker createMyWorker() {
        ExecutorService executorService = createExecutorService(); // returns a new ThreadPoolExecutor
        return new MyBean(executorService);
    }
}
public class MyWorker {
    ...
    public boolean onShutdown() throws InterruptedException {
        this.executorService.shutdown();
        boolean terminatedGracefully = executorService.awaitTermination(180, TimeUnit.SECONDS);
        return terminatedGracefully;
    }
}

How to reproduce?
Unfortunately, I cannot reproduce it.

@dzoech dzoech added the Bug For general bugs on Jetty side label Feb 24, 2022
@dzoech
Copy link
Contributor Author

dzoech commented Oct 17, 2022

This issue is still occurring for us. Is there anything more I can do to help here?

@joakime
Copy link
Contributor

joakime commented Oct 17, 2022

Wonder how a null job even entered the queue.

We have handling for null on _jobs.poll() in other places in QTP, why not here?

@joakime
Copy link
Contributor

joakime commented Oct 17, 2022

Yeah, if you have a QueuedThreadPool, and you attempt to execute/offer a null job you'll get an exception.

QueuedThreadPool pool = new QueuedThreadPool();
pool.start();
pool.execute(null);
java.lang.NullPointerException
	at java.base/java.util.Objects.requireNonNull(Objects.java:221)
	at org.eclipse.jetty.util@10.0.13-SNAPSHOT/org.eclipse.jetty.util.BlockingArrayQueue.offer(BlockingArrayQueue.java:391)
	at org.eclipse.jetty.util@10.0.13-SNAPSHOT/org.eclipse.jetty.util.thread.QueuedThreadPool.execute(QueuedThreadPool.java:756)

@dzoech
Copy link
Contributor Author

dzoech commented Oct 18, 2022

Would extending the if condition like this

else if (job != NOOP || job != null) {
    LOG.warn("Stopped without executing or closing {}", job);
}

be sufficient here? I can open a PR if you like.

@joakime
Copy link
Contributor

joakime commented Oct 19, 2022

@gregw & @sbordet WDYT?

@sbordet
Copy link
Contributor

sbordet commented Nov 22, 2022

I think the loop should not test for isEmpty(), but just trying to poll() until it sees null -- a job can be stolen while stopping.

@dzoech care to submit a pull request?

@dzoech
Copy link
Contributor Author

dzoech commented Nov 24, 2022

Sure, I'll follow up with a PR in the next couple of weeks.

dzoech added a commit to dzoech/jetty.project that referenced this issue Feb 3, 2023
Signed-off-by: Dominik Zöchbauer <dominik@zoechbauer.info>
dzoech added a commit to dzoech/jetty.project that referenced this issue Feb 3, 2023
Signed-off-by: Dominik Zöchbauer <dominik@zoechbauer.info>
dzoech added a commit to dzoech/jetty.project that referenced this issue Feb 3, 2023
Signed-off-by: Dominik Zöchbauer <dominik@zoechbauer.info>
dzoech added a commit to dzoech/jetty.project that referenced this issue Feb 3, 2023
Signed-off-by: Dominik Zöchbauer <dominik@zoechbauer.info>
@dzoech
Copy link
Contributor Author

dzoech commented Feb 6, 2023

I now created the PR against 9.4 since we are using this version, which is predefined by org.springframework.boot:spring-boot-dependencies:2.7.4.

Is this the correct approach to get it merged?

@sbordet
Copy link
Contributor

sbordet commented Feb 6, 2023

@dzoech please make the PR against branch jetty-10.0.x.

Jetty 9.4.x is at End of Community Support, see #7958.

dzoech added a commit to dzoech/jetty.project that referenced this issue Feb 7, 2023
Signed-off-by: Dominik Zöchbauer <dominik@zoechbauer.info>
sbordet added a commit that referenced this issue Feb 8, 2023
* Issue #7650 - Fix race condition when stopping QueuedThreadPool

Signed-off-by: Dominik Zöchbauer <dominik@zoechbauer.info>
Co-authored-by: Greg Wilkins <gregw@webtide.com>
Co-authored-by: Simone Bordet <simone.bordet@gmail.com>
gregpoulos pushed a commit to gregpoulos/jetty.project that referenced this issue Feb 9, 2023
…x-documentation-operations-logging

* upstream/jetty-12.0.x: (35 commits)
  Fixes jetty#9326 - Rename DecryptedEndPoint to SslEndPoint.
  Jetty 10 Upgrade to Hazelcast 5 and totally disable auto join multicast etc.. (fix build on CI) (jetty#9331)
  jetty#9328 - changes from review
  jetty#9287 - catch error in ee9 maxRequestSize MultiPart test
  Jetty 12.0.x 9301 fix ee10 jstl jpms (jetty#9321)
  Issue jetty#9301 Fix dependencies for ee10-glassfish-jstl module (jetty#9303)
  Jetty 12 Hazelcast 5.x and disable auto detection/multicast" (jetty#9332)
  jetty#9287 - fix further test failures
  Fixed imports.
  Issue jetty#7650 - Fix race condition when stopping QueuedThreadPool (jetty#9325)
  jetty#9287 - remove unpaired release of Content.Chunk
  Issue jetty#8991 - rename websocket isDemanding() method to isAutoDemanding()
  Issue jetty#9287 - fix failing tests
  changes f rom review
  add todo to revert to normal pool after fix for jetty#9311
  Issue jetty#9309 - Introducing test for requestlog format with spaces
  use non-pooling RetainableByteBufferPool to work around performance bug
  consumeAvailable should use number of reads instead of bytes
  fix for retainable merge
  changes from review
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants