AbstractGetMapProducer's instance variables not thread safe -- race condition exists when set by GetMapResponse
---------------------------------------------------------------------------------------------------------------
Key: GEOS-3679
URL: http://jira.codehaus.org/browse/GEOS-3679
Project: GeoServer
Issue Type: Bug
Components: Imagemap
Affects Versions: 2.0.0
Environment: All
Reporter: David Niedzielski
Assignee: Andrea Aime
GetMapResponse.execute() (getMapResponse.java) locates a delegate for a particular mime type via getDelegate() around line 156, which returns the same handler instance across all GetMap requests with the same outputFormat. Next, a WMSMap instance is created to represent the specifics of the particular request, and this object is passed to the delegate's setMapContext() function around line 161. Later in the code, at around line 424, the delegate's produceMap function is called to actually create the map. However, if multiple threads are executing this code simultaneously, a race condition map arise if multiple GetMap request requesting the same output format execute this code nearly simultaneously. The default setMapContext() is implemented in AbstractGetMapProducer by simply storing it in an instance variable. But because the same instance of an AbstractGetMapProducer extension (such as HTMLImageMapMapProducer) is shared for all requests with the same outputFormat, one thread can set the context, then another can overwrite it. When the first thread finally gets to call produceMap(), the context belongs to the request handled on the second thread (similar exposures exist with other instance variables stored by AbstractGetMapProducer). The same problem exists in HTMLImageMapMapProducer, where the member variable htmlImageMapEncoder is overwritten by competing threads handling GetMap requests with the text/html mime type. I have implemented a fix using ThreadLocal storage to address these issues, and will gladly provide a patch upon request. I would appreciate your opinion on these "issue". Was the intent perhaps to have each extension of AbstractGetMapProducer re-implement these getter/setters in a thread-safe way? I hope I have not mis-understood the code, and apologize in advance if that is the case!
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://jira.codehaus.org/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira