RFR: 8232853: AuthenticationFilter.Cache::remove may throw ConcurrentModificationException

[Daniel Fuchs]

Thanks for this fix Julia!

WRT to the test, I'd suggest this to make it more maintainable:

         static final int REQUEST_COUNT = 5;
         static final int URI_COUNT     = 6;
  67     static final CyclicBarrier barrier =
              new CyclicBarrier(REQUEST_COUNT * URI_COUNT);


  257     void doClient(List<URI> uris) {
              assert uris.size() == URI_COUNT;
  258         barrier.reset();
  ...
  263         for (int i = 0; i < REQUEST_COUNT; i++) {

this should ensure that the number of parties involved in the
cyclic barrier remain consistent with the number of requests that
the test sends concurrently.

best regards,

-- daniel

On 01/11/2019 16:48, Julia Boes wrote:
> Hi,
> 
> This fix addresses an issue in AuthenticationFilter.Cache::remove that 
> can lead to a ConcurrentModificationException. The cache is implemented 
> as LinkedList, the method iterates over the list and potentially removes 
> items without using an iterator.
> 
> To reproduce the bug, multiple requests are sent asynchronously so that 
> successful response headers might arrive concurrently and compete in 
> updating the cache. This increases the chances to trigger the removal of 
> a previously cached credential while storing the credentials of the next 
> response. Under these circumstances, ConcurrentModificationException is 
> thrown with very high frequency (if not always).
> 
> The proposed fix replaces LinkedList with CopyOnWriteArrayList to 
> preclude the exception.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8232853
> 
> Webrev: 
> http://cr.openjdk.java.net/~jboes/webrevs/8232853/webrev.01/index.html
> 
> The test fails reliably without the fix, and passes with it (jdk_net 
> tests run 50 times).
> 
> 
> Regards,
> 
> Julia
> 
>