jayadeep-jayaraman commented on code in PR #50020:
URL: https://github.com/apache/spark/pull/50020#discussion_r1981412176


##########
core/src/main/scala/org/apache/spark/BarrierCoordinator.scala:
##########
@@ -80,8 +81,9 @@ private[spark] class BarrierCoordinator(
       states.forEachValue(1, clearStateConsumer)
       states.clear()
       listenerBus.removeListener(listener)
-      ThreadUtils.shutdown(timer)
     } finally {
+      timerFuture.foreach(_.cancel(true))
+      ThreadUtils.shutdown(timer)

Review Comment:
   Sorry, can you please explain what do you mean by _not do both_ ? The issue 
is `timer` is a **non-daemon thread** which does not allow the JVM to exit on 
the completion of the application, this specific change handles the 
cancellation of the thread. I am not sure how daemon thread is applicable here. 
   
   Are you recommending that I convert the `timer` thread from 
`newSingleThreadScheduledExecutor` to `newDaemonSingleThreadScheduledExecutor` ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to