NPE in AtmosphereRequestImpl because of race condition

[mperktold]

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: https://github.com/Atmosphere/atmosphere/blob/24a1456c137e36dfe7c7a6b180ebe299713fb457/modules/cpr/src/main/java/org/atmosphere/cpr/AtmosphereRequestImpl.java#L393-L401 you see that this can only happen due to race conditions. It first checks whether 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.

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

Atmosphere version

• Atmosphere version 3.0.3.slf4jvaadin2 (Over Vaadin Flow)

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.

Systems (please complete the following information):

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

Additional context I also posted this issue here.