Interrupt flag is not always cleared in between requests

[arhimondr]

Jetty version(s)

9.4.44.v20210927

Java version/vendor (use: java -version)

openjdk version "11.0.14" 2022-01-18 LTS
OpenJDK Runtime Environment Zulu11.54+23-CA (build 11.0.14+9-LTS)
OpenJDK 64-Bit Server VM Zulu11.54+23-CA (build 11.0.14+9-LTS, mixed mode)

OS type/version

Linux fedora 5.16.5-200.fc35.x86_64 #1 SMP PREEMPT Tue Feb 1 21:37:11 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Description

Sometimes when a thread processing a request is interrupted the "interrupted" flag is carried over to the next request handled by the same thread.

How to reproduce?

1. Interrupt a thread while it is processing Request 1
2. Send Request 2. When Request 2 is processed by the same thread check if the current thread is interrupted.

Unfortunately the issue doesn't reproduce all the time.

[arhimondr]

Example stack trace: https://gist.github.com/arhimondr/81d80f27722d5429bf5e210034660369

In QueuedThreadPool the interrupt flag seem to be properly cleared: https://github.com/eclipse/jetty.project/blob/6c79be91b74ccb5e9fa7fecc7adb1daff9219dc4/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java#L1055

I wonder if a single job in the QueuedThreadPool can be processing more than a single HTTP request?

[sbordet]

@arhimondr like you noticed, we reset the interrupted flag for every job we run.

Jetty calls Thread.interrupt() only when it's being stopped, so could this be a race of Jetty being stopped while the application is still processing a request?

I wonder if a single job in the QueuedThreadPool can be processing more than a single HTTP request?

That is not the case.

[arhimondr]

so could this be a race of Jetty being stopped while the application is still processing a request?

It shouldn't be the case.

That is not the case.

@sbordet Does anything change for non blocking (async) requests?

It looks super weird. I will try to create a simple reproducer that I can share.

[arhimondr]

@sbordet

Actually I'm able to reproduce the problem with something as simple as

public class TestingServlet
            extends HttpServlet
    {
        @Override
        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
                throws IOException
        {
            if (Thread.currentThread().isInterrupted()) {
                resp.sendError(555);
                return;
            }
            Thread.currentThread().interrupt();
        }
    }

When sending requests repeatedly (even with a simple single threaded loop) some of them return status 555.

[joakime]

Need more details on how you are using/deploying this servlet.
It matters.

Example:
If you are using a ServletContextHandler or embedded mode.

First time through on a REQUEST dispatch you hit Thread.currentThread().interrupt().
Which causes an error.
Now you have an ERROR dispatch to the same servlet.
You now have the flow through Thread.currentThread.isInterrupted(),
But you call resp.sendError() on a an ERROR dispatch. (that's not supported).
Thread comes back to QTP and is cleaned up properly.

[arhimondr]

@joakime Here is a different reproducer without sendError involved:

private static class TestingServlet
            extends HttpServlet
    {
        @Override
        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
                throws IOException
        {
            try (Writer writer = resp.getWriter()) {
                boolean interrupted = Thread.currentThread().isInterrupted();
                writer.write(interrupted ? "interrupted" : "not interrupted");
            }
            Thread.currentThread().interrupt();
        }
    }

The reproducer is based on Airlift that uses Jetty. Here is the full implementation: https://gist.github.com/arhimondr/873aee23135e538c0a4d5f7735db1b98

This class initializes the server: https://github.com/airlift/airlift/blob/master/http-server/src/main/java/io/airlift/http/server/HttpServer.java

[sbordet]

I think the problem is that we clear the interrupted flag in QueuedThreadPool, but not in ReservedThread.

[gregw]

I'm wondering if we should also clear in HttpChannel.unhandle, as we can iterate over several calls to the application there ( eg error dispatch) and we should probably not prepared interrupt status.... or should we... as it is for the same request???