Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/20297#discussion_r162851582
  
    --- Diff: 
launcher/src/main/java/org/apache/spark/launcher/ChildProcAppHandle.java ---
    @@ -48,14 +48,16 @@ public synchronized void disconnect() {
     
       @Override
       public synchronized void kill() {
    -    disconnect();
    -    if (childProc != null) {
    -      if (childProc.isAlive()) {
    -        childProc.destroyForcibly();
    +    if (!isDisposed()) {
    +      setState(State.KILLED);
    --- End diff --
    
    Even the order doesn't matter, I think it's more conventional to set the 
state at the end.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to