issues/100: Child processes can't get the parent FDs before they're closed #101
Conversation
| @@ -115,6 +116,71 @@ func (d *Context) parent() (child *os.Process, err error) { | |||
| d.rpipe.Close() | |||
| encoder := json.NewEncoder(d.wpipe) | |||
| err = encoder.Encode(d) | |||
|
|
|||
| // wait for worker to start running or else network calls fail: https://github.com/sevlyar/go-daemon/issues/100 | |||
| time.Sleep(2 * time.Second) | |||
There was a problem hiding this comment.
Thank you for the contribution.
The solution is not stable and makes the bug is noise. Any delay leaves chance for reproducing of the bug.
We should implement child-parent processes syncronization to fix the bug properly. Child process can write an acknowledgment to its parent after initialization and parent can wait the acknowledgment.
There was a problem hiding this comment.
Hey @sevlyar, sorry about the extra changes. Forgot to remove those.
I tried child parent synchronization and it still doesn't work 😞
It seems like it has to have some sort of activity in worker happen first before parent exits or it doesn't let anything else happen.
But! I likely didn't do it right. Are you able to help me with the synchronization? I am unfortunately fairly new to golang and not a pro at this whole os process stuff.
There was a problem hiding this comment.
To clarify a bit: It looks like the child finishes initializing wayy before the worker starts working. I used prints to track what executes when to confirm when I first tried parent/child sync.
| var initialized = false | ||
|
|
||
| func (d *Context) child() (err error) { | ||
| if initialized { | ||
| return os.ErrInvalid | ||
| } | ||
| initialized = true | ||
|
|
||
| decoder := json.NewDecoder(os.Stdin) | ||
| if err = decoder.Decode(d); err != nil { | ||
| return | ||
| } | ||
|
|
||
| // create PID file after context decoding to know PID file full path. | ||
| if len(d.PidFileName) > 0 { | ||
| d.pidFile = NewLockFile(os.NewFile(4, d.PidFileName)) | ||
| if err = d.pidFile.WritePid(); err != nil { | ||
| return | ||
| } | ||
| defer func() { | ||
| if err != nil { | ||
| d.pidFile.Remove() | ||
| } | ||
| }() | ||
| } | ||
|
|
||
| if err = syscallDup(3, 0); err != nil { | ||
| return | ||
| } | ||
|
|
||
| if d.Umask != 0 { | ||
| syscall.Umask(int(d.Umask)) |
There was a problem hiding this comment.
Looks like the PR contains a lot of changes. And i should review all of them. But really the PR contains the one-line fix and other it is just moves of the old code. Please return it back or provide the refactoring (code moves) in a separate PR with appropriate comment.
|
Hey @sevlyar , what do you want to do with this? This workaround seems to solve the issue in a production grade application we built, but I'm sure there is a better way of doing it. |
Context: #100