Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dcrdtest: avoid concurrent exec.Cmd.Wait calls #22

Merged
merged 1 commit into from
Apr 4, 2024
Merged

Conversation

jrick
Copy link
Member

@jrick jrick commented Apr 4, 2024

exec.Cmd.Wait is not safe to call concurrently. Instead, call this method
only once, in the goroutine started by node.start() used for abnormal process
exit. After Wait exits, assign the error and close a new cmdDone channel
added to the node struct. This allows node.stop() to detect when process
shutdown has occurred, and provides access to the error so it can be logged.

Since some other parts of node.stop() were racy on reading node.cmd, and it
was unclear how it was intended to handle the difference between a node that
was never successfully started and one that was previously stopped, another
new channel cmdStarted is added to node to distinguish these conditions.

exec.Cmd.Wait is not safe to call concurrently.  Instead, call this method
only once, in the goroutine started by node.start() used for abnormal process
exit.  After Wait exits, assign the error and close a new cmdDone channel
added to the node struct.  This allows node.stop() to detect when process
shutdown has occurred, and provides access to the error so it can be logged.

Since some other parts of node.stop() were racy on reading node.cmd, and it
was unclear how it was intended to handle the difference between a node that
was never successfully started and one that was previously stopped, another
new channel cmdStarted is added to node to distinguish these conditions.
@jrick jrick changed the title dcrdtest: call os.Process.Wait during abnormal shutdown detection dcrdtest: avoid concurrent exec.Cmd.Wait calls Apr 4, 2024
@davecgh davecgh merged commit 9eed401 into decred:master Apr 4, 2024
6 checks passed
@jrick jrick deleted the race branch April 4, 2024 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants