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

use signal.signal() on Windows #1430

Merged
merged 1 commit into from
Nov 1, 2024
Merged

Conversation

zrquan
Copy link
Contributor

@zrquan zrquan commented Nov 1, 2024

Fix #1423

@maurosoria maurosoria merged commit f1a1d30 into maurosoria:master Nov 1, 2024
9 checks passed
@shelld3v
Copy link
Collaborator

shelld3v commented Nov 7, 2024

@zrquan Hey are you sure this works well? Cause I thought back then that if we used signal.signal then all the exception handlers wouldn't receive the exception calls from self.handle_pause right? Even though it did work well in my tests but I still couldn't figure out why it worked and would there be any issue.

@shelld3v
Copy link
Collaborator

shelld3v commented Nov 7, 2024

And if we can make sure that signal.signal works perfectly well then I don't see why we shouldn't do the same to Linux and even the default "threads" mode

zrquan added a commit to zrquan/dirsearch that referenced this pull request Nov 13, 2024
@zrquan
Copy link
Contributor Author

zrquan commented Nov 13, 2024

@shelld3v You’re right, this doesn’t work on Windows—I got a bit lazy and didn’t test it there since I’m on Linux. 😰

do the same to Linux and even the default "threads" mode

I submitted a new PR #1439 following your advice.

@zrquan
Copy link
Contributor Author

zrquan commented Nov 13, 2024

And I found a bug in "threads" mode: when skipping the current directory to the next one, the program just exits.

Screencast_20241113_224005.mp4

@shelld3v
Copy link
Collaborator

@zrquan Yeah, I think exceptions raised in handle_pause can't be catched in start, so probably by far we should only use signal.signal in async mode. Bringing it to threads mode will take more than just a few lines of code

@zrquan
Copy link
Contributor Author

zrquan commented Nov 15, 2024

I think exceptions raised in handle_pause can't be catched in start

I tested it on Linux, and it can catch exceptions just fine.

The bug I mentioned earlier happens because the "next" operation sets the fuzzer's quit_event, but it isn't cleared in the next scan, which stops the task. I fixed it in commit 82eeb84.

maurosoria added a commit that referenced this pull request Dec 18, 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.

Async mode doesn't work well on Windows
3 participants