jetty / jetty.project

Eclipse Jetty® - Web Container & Clients - supports HTTP/2, HTTP/1.1, HTTP/1.0, websocket, servlets, and more
https://eclipse.dev/jetty
Other
3.83k stars 1.91k forks source link

NPE in `HttpField.getHeader()` when `ServletChannelState.asyncError()` is called by H2 #11785

Open lorban opened 4 months ago

lorban commented 4 months ago

Jetty version(s) 12.0.9

Jetty Environment EE10

Description

There seems to be a race condition in ServletChannelState.asyncError() that sometimes ends up making HttpField.getHeader() throw a NPE:

java.lang.NullPointerException: Cannot invoke "org.eclipse.jetty.http.HttpField.getHeader()" because "field" is null
    at org.eclipse.jetty.server.internal.HttpChannelState$ErrorResponse.getResponseHttpFields(HttpChannelState.java:1636) ~[jetty-server-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.server.internal.HttpChannelState$ChannelResponse.<init>(HttpChannelState.java:1104) ~[jetty-server-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.server.internal.HttpChannelState$ErrorResponse.<init>(HttpChannelState.java:1623) ~[jetty-server-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.server.internal.HttpChannelState$ChannelCallback.failed(HttpChannelState.java:1562) ~[jetty-server-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.ee10.servlet.ServletChannel.onCompleted(ServletChannel.java:767) ~[jetty-ee10-servlet-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.ee10.servlet.ServletChannel.handle(ServletChannel.java:426) ~[jetty-ee10-servlet-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.server.handler.ContextHandler$ScopedContext.run(ContextHandler.java:1292) ~[jetty-server-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.server.handler.ContextHandler$ScopedContext.run(ContextHandler.java:1285) ~[jetty-server-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.ee10.servlet.ServletChannelState.runInContext(ServletChannelState.java:1289) ~[jetty-ee10-servlet-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.ee10.servlet.ServletChannelState.asyncError(ServletChannelState.java:858) ~[jetty-ee10-servlet-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.server.handler.ContextHandler$ScopedContext.accept(ContextHandler.java:1273) ~[jetty-server-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.server.handler.ContextRequest.lambda$addFailureListener$1(ContextRequest.java:53) ~[jetty-server-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.server.internal.HttpChannelState.lambda$onFailure$2(HttpChannelState.java:446) ~[jetty-server-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.util.thread.SerializedInvoker$Link.run(SerializedInvoker.java:191) ~[jetty-util-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.http2.server.internal.HttpStreamOverHTTP2.lambda$onFailure$1(HttpStreamOverHTTP2.java:594) ~[jetty-http2-server-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.runTask(AdaptiveExecutionStrategy.java:478) ~[jetty-util-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.consumeTask(AdaptiveExecutionStrategy.java:441) ~[jetty-util-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.tryProduce(AdaptiveExecutionStrategy.java:293) ~[jetty-util-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.run(AdaptiveExecutionStrategy.java:201) ~[jetty-util-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:311) ~[jetty-util-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:979) ~[jetty-util-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.doRunJob(QueuedThreadPool.java:1209) ~[jetty-util-12.0.9.jar:12.0.9]
    at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1164) ~[jetty-util-12.0.9.jar:12.0.9]
    at java.base/java.lang.Thread.run(Thread.java:1583) ~[na:na]
    Suppressed: java.lang.NullPointerException: Cannot invoke "org.eclipse.jetty.http.HttpField.getHeader()" because "f" is null
        at org.eclipse.jetty.http.HttpFields$Mutable.remove(HttpFields.java:1437) ~[jetty-http-12.0.9.jar:12.0.9]
        at org.eclipse.jetty.ee10.servlet.ServletContextResponse.resetContent(ServletContextResponse.java:385) ~[jetty-ee10-servlet-12.0.9.jar:12.0.9]
        at org.eclipse.jetty.ee10.servlet.ServletChannel.handle(ServletChannel.java:517) ~[jetty-ee10-servlet-12.0.9.jar:12.0.9]
        ... 18 common frames omitted
        Suppressed: org.eclipse.jetty.io.EofException: Reset cancel_stream_error
            at org.eclipse.jetty.http2.server.HTTP2ServerConnectionFactory$HTTPServerSessionListener.onReset(HTTP2ServerConnectionFactory.java:158)
            at org.eclipse.jetty.http2.HTTP2Stream.notifyReset(HTTP2Stream.java:876)
            at org.eclipse.jetty.http2.HTTP2Stream.onReset(HTTP2Stream.java:586)
            at org.eclipse.jetty.http2.HTTP2Stream.process(HTTP2Stream.java:357)
            at org.eclipse.jetty.http2.HTTP2Session.onReset(HTTP2Session.java:346)
            at org.eclipse.jetty.http2.HTTP2Connection.onReset(HTTP2Connection.java:259)
            at org.eclipse.jetty.http2.parser.BodyParser.notifyReset(BodyParser.java:139)
            at org.eclipse.jetty.http2.parser.ResetBodyParser.onReset(ResetBodyParser.java:94)
            at org.eclipse.jetty.http2.parser.ResetBodyParser.parse(ResetBodyParser.java:61)
            at org.eclipse.jetty.http2.parser.Parser.parseBody(Parser.java:229)
            at org.eclipse.jetty.http2.parser.Parser.parse(Parser.java:156)
            at org.eclipse.jetty.http2.parser.ServerParser.parse(ServerParser.java:121)
            at org.eclipse.jetty.http2.HTTP2Connection$HTTP2Producer.produce(HTTP2Connection.java:349)
            at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.produceTask(AdaptiveExecutionStrategy.java:512)
            at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.tryProduce(AdaptiveExecutionStrategy.java:258)
            ... 6 common frames omitted
            Suppressed: java.lang.NullPointerException: Cannot invoke "org.eclipse.jetty.http.HttpField.getHeader()" because "f" is null
                at org.eclipse.jetty.http.HttpFields$Mutable.remove(HttpFields.java:1437)
                at org.eclipse.jetty.ee10.servlet.ServletContextResponse.resetContent(ServletContextResponse.java:385)
                at org.eclipse.jetty.ee10.servlet.ServletChannel.handle(ServletChannel.java:458)
                at org.eclipse.jetty.server.handler.ContextHandler$ScopedContext.run(ContextHandler.java:1292)
                at org.eclipse.jetty.server.handler.ContextHandler$ScopedContext.run(ContextHandler.java:1285)
                at org.eclipse.jetty.ee10.servlet.ServletChannelState.runInContext(ServletChannelState.java:1289)
                at org.eclipse.jetty.ee10.servlet.ServletChannelState.asyncError(ServletChannelState.java:858)
                at org.eclipse.jetty.server.handler.ContextHandler$ScopedContext.accept(ContextHandler.java:1273)
                at org.eclipse.jetty.server.handler.ContextRequest.lambda$addFailureListener$1(ContextRequest.java:53)
                at org.eclipse.jetty.server.internal.HttpChannelState.lambda$onFailure$2(HttpChannelState.java:446)
                at org.eclipse.jetty.util.thread.SerializedInvoker$Link.run(SerializedInvoker.java:191)
                at org.eclipse.jetty.http2.server.internal.HttpStreamOverHTTP2.lambda$onFailure$1(HttpStreamOverHTTP2.java:594)
                at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.runTask(AdaptiveExecutionStrategy.java:478)
                at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.consumeTask(AdaptiveExecutionStrategy.java:441)
                at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.tryProduce(AdaptiveExecutionStrategy.java:293)
                ... 6 common frames omitted
wendigo commented 1 month ago

Is this targeting 12.0.12 at this stage?

lorban commented 1 month ago

@wendigo we never managed to reproduce this problem, and the original reporter told us he did not see it happening again after upgrading to 12.0.9.

We still need to investigate this further to understand what the actual cause could have been, but this is a low-priority task as we are not aware of anyone being impacted by this problem.

lorban commented 1 month ago

@wendigo have you been hit by this problem, or are you just reviewing known Jetty issues that may impact you?

I'm asking because if you need some help in that area, the sooner we know about it, the better we can prioritize our tasks to make sure you don't have to wait for too long to provide you with a solution.

Thanks!

wendigo commented 1 month ago

@lorban yeah, we saw that in our benchmarks

gregw commented 2 weeks ago

@lorban The HttpFields class is not intended to be Thread safe, so a clear() from one thread whilst another is iterating may well result in these kinds of NPEs.

Several ways forward: 0) leave HttpFields as is and look really hard for the concurrent access and fix it. 1) harden HttpFields a bit for concurrent access (e.g. set size to 0 before assigning new array) and make a good effort to fix the actual concurrent access 3) Make HttpFields thread safe with concurrent list of fields and let the concurrent access remain.

Let's hangout to discuss.

Eitherway, I'm bumping to the next release.