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

Add a check to see if the node server is down or not #46

Merged
merged 1 commit into from
May 12, 2015

Conversation

wchen-r7
Copy link
Contributor

This PR is related to issue #40 (Node server shuts down and cannot restart). I haven't got a chance to actually look into why the node server goes down in some cases, but I feel it is definitely necessary for grinder to be able to restart the node server as long as the debugger is alive. If not, the debugging will just keep respawning the browser without any testcases.

Please note that I am not 100% familiar with grinder, so I'm not super confident if this implementation is appropriate or not. If you don't like it, please feel free to close.

This will check if the server is still up or not. If it's down
for some reason, it will reset the pid, and that will allow
grinder to restart the server. If the server doesn't restart,
the fuzzer will still keep going (without any testcases), and
that is bad.
@stephenfewer
Copy link
Owner

hey @wchen-r7 thanks for the patch, I'm going to run this on some of my nodes for a few days to test it out before merging. I still haven't found the root cause of #40 unfortunately.

@wchen-r7
Copy link
Contributor Author

Ok cool, no problem. Thanks.

@mutfuzz
Copy link
Contributor

mutfuzz commented May 12, 2015

so i have tested this fix for the last 2~3 months on my nodes and it is working great, i no longer see dead server

please merger this PR.

stephenfewer added a commit that referenced this pull request May 12, 2015
Add a check to see if the node server is down or not
@stephenfewer stephenfewer merged commit a9f3a0a into stephenfewer:master May 12, 2015
@stephenfewer
Copy link
Owner

Awesome, thanks @wchen-r7 for the patch and thanks @mutfuzz for helping test and confirm the results!

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.

3 participants