NPE in Atmosphere because of race condition

[mperktold]

trafficstars

Description of the bug

We got the following intermittent failure several times:

java.lang.NullPointerException: Cannot load from object array because the return value of "java.util.Map.get(Object)" is null
	at org.atmosphere.cpr.AtmosphereRequestImpl.getParameter(AtmosphereRequestImpl.java:399)
	at jakarta.servlet.ServletRequestWrapper.getParameter(ServletRequestWrapper.java:150)
	at com.vaadin.flow.server.VaadinService.findUI(VaadinService.java:1141)
	at com.vaadin.flow.server.communication.PushHandler.handleConnectionLost(PushHandler.java:395)
	at com.vaadin.flow.server.communication.PushHandler.connectionLost(PushHandler.java:340)
	at com.vaadin.flow.server.communication.PushAtmosphereHandler.onStateChange(PushAtmosphereHandler.java:62)
	at org.atmosphere.cpr.DefaultBroadcaster.invokeOnStateChange(DefaultBroadcaster.java:1036)
	at org.atmosphere.cpr.DefaultBroadcaster.prepareInvokeOnStateChange(DefaultBroadcaster.java:1056)
	at org.atmosphere.cpr.DefaultBroadcaster.executeAsyncWrite(DefaultBroadcaster.java:870)
	at org.atmosphere.cpr.DefaultBroadcaster$2.run(DefaultBroadcaster.java:477)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source)
	at java.base/java.util.concurrent.FutureTask.run(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool.scan(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source)

Looking at the code of AtmosphereRequestImpl.getParameter, you see that this can only happen due to race conditions. It checkes first to see if the parameter is in the map, and if it is, instead of using the retrieved value, it retrieves it again. In a multi-threading scenario, the parameter could be removed in between those instructions.

The whole class seems to be conscious about multi-threading. For example, the queryStrings Map is a synchronized Map. So it's clearly a bug to access this map in such a "check-then-act" way.

Expected behavior

The getParameter method should access the map only once and use the result. In general, the class should be threadsafe and not throw NPEs in certain multi-threading scenarios.

Minimal reproducible example

I don't have a minimal example, as I'm also not sure whether this is specific to the application.

Versions

• Vaadin / Flow version: 24.1.10
• Java version: 17.0.8
• OS version: Windows 10
• Application Server: Jetty 11.0.17

[mshabarov]

The issue seems to be in org.atmosphere.cpr.AtmosphereRequestImpl class itself, but we can try to workaround this in the Vaadin Atmosphere handler.

[mperktold]

I can also forward this to atmosphere if you prefer.

[mshabarov]

Yes, this makes sense to me at least ask them. Would be a better way if they make their implementation a thread-safe.