Fix sighup handler [T2625] #557
Conversation
add integration tests
| } | ||
| log.Debugf("Forking new process of %s", ServiceName) | ||
|
|
||
| binaryPath, err := exec.LookPath(os.Args[0]) |
There was a problem hiding this comment.
Are we sure (and should we) that the binaryPath is the same executable as the current Acra instance? I can image situations, when someone puts another acra executable earlier in PATH, so after the sighup another binary will be called. But, is it worth to worry about?
There was a problem hiding this comment.
no, we don't sure. Golang doesn't fork syscall due to complicated runtime and fork limitations - https://stackoverflow.com/questions/28370646/how-do-i-fork-a-go-process
execve* requires path to executable that should be loaded. We try to find executable using same ENV variables used to run process itself. But yes, if we have PATH=/dir1:/dir2 and acra-server was loaded from /dir2/acra-server and before sighup somebody place acra-server in /dir1 then we will load new binary.
Zhaars
left a comment
Zhaars
left a comment
There was a problem hiding this comment.
Everything is clear to me
Found that process reloading on SIGHUP stopped properly handling for acra-server and acra-translator. Previously it used first argument of the process which is process's name. Under the hood golang calls execve which doesn't search binary in PATH. Linux has execlp(), execvp(), and execvpe() functions for that. But instead of using raw syscall's we use golang's wrappers from syscall package which doesn't provide these functions. Otherwise it has portability and do other useful things like child checking, lock handling, etc
So, in this PR added searching absolute path of binary from binary name before calling ForkExec. Additionally added integration tests that checks sockets forwarding to child process and pulling config_file changes on SIGHUP.
Checklist
with new changes