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

Reply via email to