TEZ-4277: AsyncDispatcher can hang on serviceStop if the eventhandling thread is in BLOCKED state #97
Conversation
abstractdog
changed the title
…venthandling thread is in BLOCKED state Change-Id: I1418c9dc9972d2231d961afd41f306307754cabd
| } | ||
| stopped = true; | ||
| if (eventHandlingThread != null) { | ||
| eventHandlingThread.interrupt(); |
There was a problem hiding this comment.
Thanks for sharing the patch. Overall looks good. Minor comments below.
If you look at eventHandlingThread, it would be making a call to "protected void dispatch(Event event)". This method catches "throwable" and does not reset Thread's interrupt status. So in case, interrupt happened during dispatch, it would be silently gobbled up.
Can you add snippet to catch "InterruptedException" in asyncDispatcher::dispatch(), log that it got interrupted and just reset thread's interrupt status with ("Thread.currentThread().interrupt();"). This would ensure that the next iteration in event handler thread gets a chance to bail out.
There was a problem hiding this comment.
thanks @rbalamohan, makes sense, I added it in a new commit
rbalamohan
left a comment
rbalamohan
left a comment
There was a problem hiding this comment.
Left minor comment in the PR.
rbalamohan
left a comment
rbalamohan
left a comment
There was a problem hiding this comment.
Added minor comment. Rest lgtm. +1
| } else { | ||
| throw new Exception("No handler for registered for " + type); | ||
| } | ||
| } catch (InterruptedException ie) { |
There was a problem hiding this comment.
Can you add this within Throwable block (with instance of checks)? Rest lgtm.
There was a problem hiding this comment.
thanks, it's done
2 participants
No description provided.