search

brettwooldridge / NuProcess

Low-overhead, non-blocking I/O, external Process implementation for Java
Apache License 2.0
710 stars 84 forks source link

Alternative fix for linux race condition between onStart and onExit. #156

Open avrecko opened 3 months ago

avrecko commented 3 months ago

Continuing from #149.

Moved onStart processing to event processing thread. This way we make sure onStart is called first before any other EPoll event processing is done.

@bturner This is just my suggestion. Curious how you would fix it.

bturner commented 3 months ago

@avrecko,

Thanks for the idea, but I'd prefer to keep it where the process manages onStart. The approach I'm leaning toward splits the existing IEventProcessor.registerProcess(T) method to extract a new queueRead (sibling to the existing queueWrite). With the split, registerProcess does the pidToProcessMap and fildesToProcessMap updates and queueRead does the epoll_ctl calls for stdout and stderr (on Linux; it does the kevent calls on macOS). This allows the LinuxProcess code to call registerProcess where it currently does, and then do a myProcessor.queueRead(this) after callStart.

I've opened pull request #158 with that approach, and I've tested it on all of the platforms I can.

avrecko commented 3 months ago

@bturner

Makes sense to do it this way. Keeping the threading model as is.

Looks good. I'd probably name the new method registerRead or something similar.

I trust the tests are now run with the build process and catch this case.