David Ribeiro Alves has posted comments on this change. Change subject: threadpool: token-based task sequencing ......................................................................
Patch Set 10: @adar i guess that the first explicit call to Shutdown() is racing with the the rest of the stuff. i.e. the token’s shutdown completes and the shutdown thread thinks it’s released but there are actually other tasks hanging on to tokens even though they haven’t yet submitted. Of course if you did let them submit they might cause SIGSEGV’s cause the pool was destroyed or somesuch. My suggestion would be: - Make ThreadPool ref counted, make tokens hold a ref. (to make sure the tp survives and to avoid adding locks inside of ThreadPoolToken). - On Token shutdown actually release the token from the pool after you’ve quiesced all the tasks(but the tokens ref for the pool will only be reduced on the dctor). - Make the pool accept but return a bad status for unknown/quiesced tokens (which I already think it does, just spelling it out). This way the shutdown thread will clear all pending tasks and make sure that the running tasks complete but if there are places still hanging on to tokens that should still be ok. -- To view, visit http://gerrit.cloudera.org:8080/6874 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If46dc34212027b6ea5dbc2ead7c7af8be57f2c70 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: No
