test to ensure map types in ProvidersRegistry
---------------------------------------------
Key: WINK-179
URL: https://issues.apache.org/jira/browse/WINK-179
Project: Wink
Issue Type: Wish
Components: Common
Reporter: Mike Rheinheimer
A question came up recently about the necessity for
ProvidersRegistry.MediaTypeMap to use type SimpleConcurrentMap since some lock
protection is already built into the ProvidersRegistry methods:
getContextResolver(), getMessageBodyReader(), or getMessageBodyWriter().
The question is, why not just use a normal HashMap instead of
SimpleConcurrentMap? It could be faster to avoid the extra locking, which
appears unnecessary here.
As it turns out, the extra locking is necessary. From Bryant:
"The reason why there are two protections is that the first protection is in
cases where the providers are dynamically added.
The second protection is for the cache itself which could be written to by two
different threads even if they both were getting a single MessageBodyReader
(i.e. a cache value may be dropped and then two threads come back later and try
to write a new cache value). Due to some weird HashMap properties, this can
blow up in weird ways."
So, my wish is to have a test that ensures the map type is appropriate and
remains appropriate. That way if someone comes along in the future and sees
the same issue, any change they make to the code will be caught in the unittest
and a careful reading the code comments and unittest would show why the
MediaTypeMap is using the types it uses.
The problem is, we need to inspect the instantiated providersCache object, and
there is currently no way to get to that object.
What's your opinion? Since this is a sensitive path, it needs to have a
unittest or two around it. Do we want to change the inner class and field
visibility just to make it testable? This is the easy way to test it; we can
make the MediaTypeMap inner class and providersCache object "protected", then
override the MediaTypeMap class in a test to implement a getProvidersCache
method so we can inspect the object (see attached patch).
I suppose the other way to test it is to build up a multi-threaded test that
dynamically add providers, the way Bryant mentioned. Sounds fun. :)
The attached patch is NOT FINAL. Please review and let's discuss. Consider it
merely a suggestion.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
