RFR: 8151299 Http client SelectorManager overwriting read and write events

[Michael McMahon]

On 08/03/16 14:10, Claes Redestad wrote:
>
>
> On 2016-03-08 14:42, Michael McMahon wrote:
>>
>>> Iterator-based removal would work, or building a new list to replace 
>>> pending with might be more efficient. The synchronization scheme 
>>> seems a bit flaky as well?
>>>
>>
>> The new code is always called from the same thread. So, I don't see 
>> an issue?
>
> Right, but it seems the SelectorAttachment can escape and be observed 
> by other threads that may have access to the SelectableChannel. I 
> don't know if this could become a problem somewhere, but it makes me a 
> bit uneasy. Maybe the attachment should be an immutable key to a map 
> of some tracking object held by and managed by the SelectorManager?
>

But, I don't see how this can happen. No thread other than the selector 
manager
accesses the SelectionKeys or selector mechanics. registration of 
channels and handling
of events all occurs in this thread.

> Also, with the assumption that the SelectorManager is running in a 
> distinct thread, couldn't this be checked at the start of run() and 
> then avoid synchronized(this) in the loop?
>

The synchronization is required for controlling access to the 
registrations List<AsyncEvent>
This is the data structure accessed by other threads.

> Another oddity in the surrounding code is that 
> System.currentTimeMillis() is always called, but only used when 
> selector.select(timeval) == 0 - could this be moved into the if-block?
>
>  266 long now = System.currentTimeMillis();
>  267                     int n = selector.select(timeval);
>  268                     if (n == 0) {
>  269                         signalTimeouts(now);
>  270                         continue;
>  271                     }
>

Yes, fair point. Will move that.

I've changed my mind on the ArrayList vs LinkedList question. I don't think
there is a significant improvement to be gained by switching to ArrayList
given that element deletions occur at much the same rate as accesses.

Thanks,
Michael.

>
> Thanks.
>
> /Claes