In org.apache.mina.transport.socket.nio.SocketIoProcessor I have a bad
_feeling_ about the concurrency of these 2 methods:

void flush(SocketSessionImpl session) {
   scheduleFlush(session);
   Selector selector = this.selector;
   if (selector != null) {
       selector.wakeup();
   }
}
void updateTrafficMask(SocketSessionImpl session) {
   scheduleTrafficControl(session);
   Selector selector = this.selector;
   if (selector != null) {
       selector.wakeup();
   }
}


I have a problem specifically with this piece of code:

Selector selector = this.selector;
if (selector != null) {
   selector.wakeup();
}

The code access 'this.selector' in an unsynchronized manner from other
threads and I don't know if some visibility/safe publication issues
might arise here.

The problem is not that you call "selector.wakeup();" from other
threads, the problem is rather the fact that you are accessing the
variable 'selector' without synchronization.

In the same class when you change the variable 'selector' you
synchronize around the 'lock' object to take care of synchronization
issues. For me that seems to indicate that on one hand you are taking
care of visibility/safe publication issues and on the other one it is
not taken care of.

If you happen to have the book "Java Concurrency in Practice", please
read again "3.5 Safe publication" on page 49-54. It seems to me that
the gray rectangle (in 3.5.3) hints that to avoid the problem we might
need to store the reference to the selector into a volatile field.

private volatile Selector selector;

What do you think? From a visibility/safe publication stand point, do
you see a problem here?

_________________________________________________________________
Ta' på udsalg året rundt på MSN Shopping: http://shopping.msn.dk - her finder du altid de bedste priser

Reply via email to