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

Fix hangs when deleting / recreating project root with the same name #548

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

misterbyrne
Copy link
Contributor

@misterbyrne misterbyrne commented Dec 4, 2017

Fix for issue detailed in #396 (comment)

If we have started a spring server, but the project root is deleted, the server can't successfully boot a child process. This is not recoverable. Even when the project is recreated, clients can also no longer communicate with the server because the socket path is derived from the project root (Dir.pwd) which for the server process no longer exists.

The proposed fix is to detect whenever a child cannot be started within a specific time frame - for whatever reason - rather than waiting indefinitely for the child to communicate with the ApplicationManager instance. This raises an error. If any error occurs when attempting to create a child process, we just abort - which shuts down the server gracefully.

This might not be a terribly common scenario, but I guess it can happen in real life - especially when people create throwaway projects to test stuff, or check out open source projects temporarily.

@rails-bot
Copy link

r? @rafaelfranca

(@rails-bot has picked a reviewer for you, use r? to override)

@@ -32,7 +32,11 @@ def restart
end

def alive?
@pid
return false if @pid.nil?
Process.getpgid(@pid)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be a reasonable way of determining whether a process with a certain pid exists.

def wait_for_child_to_boot
timeout = 1
loop do
break if IO.select([child], nil, nil, timeout)
Copy link
Contributor Author

@misterbyrne misterbyrne Dec 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The child process puts something on the socket to indicate that it's ready. Rather than simply wait forever as before (i.e. child.gets) we do the same, but check the process is still alive at 1 second intervals while doing so.

@misterbyrne
Copy link
Contributor Author

☝️ I'm not sure why CI failed. I tried a noop PR and that also failed :/

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