[THRIFT-836] Race condition causes CancelledKeyException in TAsyncClientManager

• Type: Bug
• Status: Closed
• Priority: Blocker
• Resolution: Fixed
• Fix Version/s: 0.4
• Component/s: Java - Library
• Patch Info: Patch Available
• Assignee: Bryan Duxbury
• Reporter: Ning Liang
• Created: 02/Aug/10 20:40
• Updated: 05/Aug/10 21:14
• Resolved: 05/Aug/10 21:14

Description

Currently, TAsyncClientMethod cancels its selection key on a successful method call. The current model assumes the key goes away and the client gets a new key on the next Selector registration, which is incorrect - the cancelled key can hang around, with the implication that the developer needs to check key.isValid() on the next selector action, which TAsyncClientMethod currently does. However, if the developer re-uses the client for another call, you can get a CancelledKeyException from the Selector#register call, which crashes the SelectorThread. This occurs once you have sufficient concurrency on the Selector - cancelled key cleanup takes longer, so we hit this condition.

Attached is a patch with fix and regression test. Summary of fix:

1) Don't cancel() the key after a successful method call.
2) Catch CancelledKeyException in the SelectorThread, so the manager can't die.
3) Add an onError method to TAsyncMethodCall, so that the SelectorThread can notify the client of an error on selector registration.