Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Update goDebug.js to enable the removal of the debug binary #1380

Closed
wants to merge 1 commit into from

Conversation

NexWeb
Copy link
Contributor

@NexWeb NexWeb commented Nov 29, 2017

This modification works with PR #1379, RE #1345, to enable the removal of the debug binary when pressing the stop button.

This modification works with PR microsoft#1379, RE microsoft#1345, to enable the removal of the debug binary when pressing the stop button.
@ramya-rao-a
Copy link
Contributor

I am guessing you want the killTree to be called when pressing the stop button,
And when the stop button is pressed, if the debugProcess still exists, the killTree in the else block doesnt get called.
And that's why you are changing the if condition?

@ramya-rao-a
Copy link
Contributor

Closng this PR as this change will break the original reason why the killTree was added in the first place.
When debugging is stopped, the processes spawned by the debug process were not getting terminated. That's the reason killTree was put in place.

@ramya-rao-a ramya-rao-a closed this Dec 5, 2017
@NexWeb
Copy link
Contributor Author

NexWeb commented Dec 5, 2017

This change just enables the debug binary to be deleted when a Goroutine has been started, as in the case of httpsSrv.ListenAndServe. It does not prevent the binary from being deleted in other cases.

@ramya-rao-a
Copy link
Contributor

This change reverts the fix done for #438 which repros only in Linux.
The fix for #438 was to run the killTree on the id of the debug process so that all the processes spawned from the debug process get terminated.

This change to the if condition results in the killTree never getting called on the debug process.

@NexWeb NexWeb deleted the patch-2 branch December 11, 2017 00:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants