Add Socket Manager API and Windows support #28
Conversation
| @@ -204,14 +219,31 @@ def tick(blocking_timeout=0) | |||
| return nil | |||
| end | |||
|
|
|||
| ready_pipes, _, _ = IO.select(@rpipes.keys, nil, nil, blocking_timeout) | |||
| if ServerEngine.windows? | |||
| @rpipes.each{|r, m| | |||
There was a problem hiding this comment.
That's because Win32::Pipe::Server doesn't cause EOFError even when a child process exits.
(https://github.com/fluent/serverengine/pull/26/files#diff-1e5e735f236537f81cca69fb746da179R221)
Then I think we should adapt to child process exits by Process.waitpid2 and if child process exits, we have to close pipe.
I think we can use WaitForMultipleObjects, but function is almost same and Process.waitpid2 is easier to use. Then I used that.
There was a problem hiding this comment.
By the way, win32-pipe is not stable and hard to use, because there are many bugs.
Then I think we should use UDPSocket for Heartbeat if it is allowed.
There was a problem hiding this comment.
if my understanding is correct, pipe won't be invalid even when a connecting process exists because another client can connect there. That's the reason why EOFError doesn't happen.
As I commented on #26 (comment), another idea is to not implement heartbeat at all on windows. I think it's good enough and make it simple. it mans that ProcessManager#tick does nothing excepting @monitors.delete_if {|m| !m.tick(time) } and sleep. Any thoughts there?
There was a problem hiding this comment.
I got it. I think so, too.
I fixed it.
|
@naritta Very good. Can I ask you following items to merge this awesome work?
|
naritta
force-pushed
the
socket-manager-api
branch
from
d7327d5
to
a587639
Compare
|
@frsyuki I added AppVeyor support and I fixed previous problem in travis-ci. |
This is continued from below.
#25 Sharing sockets between multiprocess workers with Socket Manager
#26 Windows support for signal handling and dealing with IO