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
