jetty.project icon indicating copy to clipboard operation
jetty.project copied to clipboard

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

Open lorban opened this issue 1 year ago • 7 comments
trafficstars

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

lorban avatar May 13 '24 12:05 lorban

Is this targeting 12.0.12 at this stage?

wendigo avatar Jul 23 '24 10:07 wendigo

@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 avatar Jul 23 '24 11:07 lorban

@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!

lorban avatar Jul 24 '24 09:07 lorban

@lorban yeah, we saw that in our benchmarks

wendigo avatar Jul 24 '24 09:07 wendigo

@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
  2. 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.

gregw avatar Sep 02 '24 06:09 gregw

@lorban I've had another look at this. I can see some ways to harden HttpFields to avoid the NPE... but I'm now convinced that is not the right thing to do. getResponseHttpFields is being called to write out a response. If we allow a parallel clear and avoid a NPE, then those response headers are not going to be correct. Depending on the race they will either have fields they should not or not have fields that they should.

The problem is the race of one thread calling clear whilst another is using it. We have to find out where that is.

gregw avatar Sep 25 '24 04:09 gregw

@wendigo do you still see this problem with the latest Jetty 12.0.13?

If we want to go to the bottom of this problem, we are going to need your help finding a way so we can reproduce this problem in our environment.

Thanks!

lorban avatar Sep 25 '24 08:09 lorban

@wendigo have you been able to reproduce this?

gregw avatar Oct 24 '24 05:10 gregw

@gregw i haven't seen it for a while. You can close this

wendigo avatar Oct 24 '24 06:10 wendigo