Heads up: SelectionKey.attach() broken ?

(nb: I’m struggling to check the bug parade on this, but it’s not searchable for the term “attach” so it’s frickin hard; c.f. http://www.java-gaming.org/cgi-bin/JGNetForums/YaBB.cgi?board=cluebies;action=display;num=1088616088)

I’m fairly confident there’s a mean, nasty, evil bug in 1.4.2 whereby SelectionKey’s arbitrarily kill their attachments, silently.

The situation I’m in, I just ran a full text search for “attach(” against all source code in the server, and verified that I have a breakpoint (or logging point) on every single line of code where it’s called, and yet I’m seeing attachments flip from a LinkedList to null without me touching them.

I think the problem might be that when you reset the interest set for a SelectionKey, sometime later (not immediately) it decides to wipe out it’s attachment. I’m trying to construct a case that proves it, but it is definitely time sensitive, which is not surprising since it’s probably triggered by the Selector doing something internally. But I might not be able to make a conclusive test case because of the timing-sensitivity :(.

Turns out it isn’t the interestOps() that kills the attachment, but instead the following.

I was re-registering SelectableChannel’s to get a new key every time they were being re-used after having an empty interestOps.

When you re-register an SC, it returns the pre-existing key.

However…the method call:

register( Selector, ops )

actually delegates to

register( Selector, ops, null )

which explicitly deletes the attachment. Which is an incredibly stupid way of designing your API - silently deleting attachments. I can’t off the top of my head think of any explanation for this behaviour other than:

  • it’s supposed to be a shorthand for killing attachments (which is stupid because that’s the kind of thing we did in C, not in Java, where things are supposed to be done cleanly)

  • the API developer was too lazy to write a two-line method body, OR didn’t stop to think what the consequences were of using null as a default (in this case, it doesn’t mean “use default” it means “delete what was already there”).

I got caught out by that one as well. The best explanation I could come up with is that its register() not reregister() :-/. I guess on the positive side, at least the javadocs are clear on this point.