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

[Alan Bateman]

On 04/11/2019 15:09, Daniel Fuchs wrote:
> :
>
>> It looks like Cache#remove can be fixed simply by using an explicit 
>> iterator and using iterator#remove instead of List#remove.
>
> Yes - we could have used an Iterator. I don't like much LinkedList.
> We could have used ArrayList and an explicit Iterator instead.
> I suggested using  CopyOnWriteArrayList just to avoid using the
> Iterator explicitly. Cache are usually few writes and multiple
> reads so that looked appropriate.
>
> Would you still advised using plain ArrayList and an Iterator instead?
I think I agree Martin in that it looks strange to use COWAL for the 
entries when all methods are synchronized. What are the access patterns 
on a Cache instance? I assume mostly get methods, in which case allowing 
concurrent access may be benefical. If there are a lot of calls to 
remove then changing it ArrayList could be O(n) due to the shifting. 
Maybe the simplest is to fix the issue at end by using the 
Iterator::remove and then re-visit the Cache implementation later (you 
mentioned other auth schemes so I assume it will need to be re-visited 
at some point anyway).

-Alan.