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

Send SIGSTOP to linux/android processes before doing other procfs/ptrace things. #108

Merged

Conversation

afranchuk
Copy link
Contributor

If this fails, we continue as we used to. This is an attempt to get a consistent/static process state.

Closes #28.

Needs to be tested in all sorts of scenarios (but due to the timeout and the fact that it continues on failure, one would hope it'd only be an improvement).

things.

If this fails, we continue as we used to. This is an attempt to get a
consistent/static process state.

Closes rust-minidump#28.
@afranchuk afranchuk requested a review from Jake-Shadle as a code owner March 20, 2024 20:36
Not sure this is needed, but better to allow users to customize this rather than rely on a hardcoded value
@Jake-Shadle Jake-Shadle merged commit a018d34 into rust-minidump:main Mar 21, 2024
14 of 15 checks passed
@gabrielesvelto
Copy link
Contributor

I had a couple of comments about this but wasn't fast enough, I'll put them in a separate issue.

@afranchuk
Copy link
Contributor Author

Yeah later last night I also realized that process_stopped should be set regardless of whether waiting for the status change succeeds.. (i.e. that needs to be fixed)

@Jake-Shadle
Copy link
Collaborator

Oh right, I thought about something like starting if the bool was true or checking of the process has reached a stopped state in the interim, but I assume that sigcont is idempotent ?

@afranchuk
Copy link
Contributor Author

afranchuk commented Mar 21, 2024

It also might not hurt to just always send SIGCONT, but it'd be cleaner to be symmetric. I think as long as the SIGSTOP goes out, there's no reason to think the process won't stop (it's unblockable after all) even if we don't observe that.

@gabrielesvelto
Copy link
Contributor

Given the processes will frequently be in the same process group it might be possible to waitpid(..., WUNTRACED) the stopped process instead of polling, and fallback on polling if that doesn't work.

@Jake-Shadle
Copy link
Collaborator

I had a couple of comments about this but wasn't fast enough, I'll put them in a separate issue.

Do you want to add yourself to https://github.com/rust-minidump/minidump-writer/blob/main/.github/CODEOWNERS so that you get auto-added as a reviewer? Then you can just remove the request if you don't have time/etc.

@gabrielesvelto
Copy link
Contributor

Do you want to add yourself to https://github.com/rust-minidump/minidump-writer/blob/main/.github/CODEOWNERS so that you get auto-added as a reviewer? Then you can just remove the request if you don't have time/etc.

Yes, thank you!

This was referenced Mar 21, 2024
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.

Improve the mechanism used to suspend threads
3 participants