In NioSocket's constructor the "this" reference is
passed to the worker's registerChannel method. As the
worker is probably running, this makes it possible for
the NioSocket object to be used before its construction
is complete, which is a very bad idea.
This effect can be minimized by making the
registerChannel call the last in the constructor, but
there are no guarantees that the compiler or JIT won't
rearrange the code, thus brining the bug back.
Besides, the attachment is added to the key after the
call, which even if the call were pulled out of the
constructor, it can cause the attachment to be
referenced by the worker (i.e., from handleRead) before
it's actually attached. This just happened to me, so
it's far from a hypothetical case. However, it's easy
to fix by passing the attachment as a parameter to
registerChannel and using the three-argument form of
SelectableChannel.register that takes the attachment.
In conclusion, there's a bug in the design,
registerChannel must be called from outside the
constructor. I'll post a patch if I fix this myself,
but that might take some time at this point.
Logged In: YES
user_id=68628
Please apply the patch I posted at
http://sourceforge.net/tracker/index.php?func=detail&aid=1187599&group_id=91167&atid=596194
to fix this issue.
The API had to be modified slightly. What before was written:
nss = new NioSocketSubclass(nioWorker, socketAddress);
now becomes:
nss = new NioSocketSubclass(socketAddress);
nss.register(nioWorker);
Additionally, now it's possible to properly hook up event
listeners for the NioSocket events without risking even more
concurrency problems. Just add your listeners before
invoking register.