Github user vanzin commented on a diff in the pull request: https://github.com/apache/spark/pull/20223#discussion_r161022567 --- Diff: launcher/src/main/java/org/apache/spark/launcher/AbstractAppHandle.java --- @@ -91,10 +92,15 @@ LauncherConnection getConnection() { return connection; } - boolean isDisposed() { + synchronized boolean isDisposed() { return disposed; --- End diff -- The `synchronized` is not protecting the variable, but all the actions that happen around the code that sets the variable. So this method returning guarantees that either all the disposal tasks have run or none have. That being said `ServerConnection.close` is not really holding the handle lock as it should, so I might have to make some changes here. These are not really contended locks. In the worst case there will be 2 threads looking the them. So trying to come up with a finer-grained locking scheme here sounds like more trouble than it's worth.
--- --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org