Github user nivox commented on a diff in the pull request:
https://github.com/apache/spark/pull/19217#discussion_r146519805
--- Diff:
launcher/src/main/java/org/apache/spark/launcher/LauncherServer.java ---
@@ -232,20 +232,20 @@ public void run() {
};
ServerConnection clientConnection = new ServerConnection(client,
timeout);
Thread clientThread = factory.newThread(clientConnection);
- synchronized (timeout) {
--- End diff --
This was exactly my reasoning. I couldn't find any reason why the
synchronisation was needed. The removal was only a means to make the code
simpler removing the cognitive cost suggested by the presence of a synchronise
keyword. The actual fix doesn't depend on it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
