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

[Julia Boes]

Thanks for the feedback everyone.


> It looks like Cache#remove can be fixed simply by using an explicit 
> iterator and using iterator#remove instead of List#remove.
>
> Cache looks fishy in other ways, e.g. why does Cache#remove ignore 
> authscheme?

I changed the implementation to use an iterator instead and check the 
authscheme:

synchronized void remove(String authscheme, URI domain, boolean proxy) {
-            for (CacheEntry entry : entries) {
-                if (entry.equalsKey(domain, proxy)) {
-                    entries.remove(entry);
+            var iterator = entries.iterator();
+            while (iterator.hasNext()) {
+                var au = iterator.next();
+                if (!Objects.equals(au.scheme, authscheme)) continue;
+                if (au.equalsKey(domain, proxy)) {
+                    iterator.remove();
                  }
              }
          }

> 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++) { 

Done.


Updated webrev: 
http://cr.openjdk.java.net/~jboes/webrevs/8232853/webrev.02/index.html

I filed a separate issue to revisit the cache implementation: 
https://bugs.openjdk.java.net/browse/JDK-8233589


Cheers,

Julia

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/net-dev/attachments/20191105/7d6cad45/attachment-0001.html>