search

brettwooldridge / NuProcess

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

FreeBSD Support - Fix syscall(SYS__pthread_chdir) failed to set current directory #141

Open fronald opened 2 years ago

fronald commented 2 years ago

Hello guys!

I am a developer and co-founder of Wiselabs Software in Brazil. We have some mission-critical systems that we chose to migrate to FreeBSD. In some of these systems we use NuProcess, and in the quality assurance process we found the following problem:

"syscall(SYS__pthread_chdir) failed to set current directory"

We also found a bug with the order in the process registration call (Kevent).

We chose to contribute to the project and resolve this bug. I hope it will be useful to you and other users.

Thank you so much for this wonderful library.

fronald commented 2 years ago

We apologize for not having noticed that there was already another PR by jasonkafer. Feel free to decline ours. Or if you prefer, we can make the changes you need.

Thanks.

bturner commented 3 months ago

Sorry for the very long delay in responding to this pull request. As was explained (by a helpful developer) on pull request #109, the change here sets the syscall to chdir, which changes the working directory for the entire process, not pthread_chdir, which changes it for the current thread. That aspect of this change won't be accepted, because using chdir has a number of issues (see this comment for details).

Adding ext to kevent may be compatible with the FreeBSD variant you're using, but is not compatible with macOS; ext is part of a separate kevent64_s struct instead (per man kqueue). We'd need some other way to handle that field to avoid memory offset issues on macOS.

I'm interested in the "bug with the order in the process registration call". Can you link me to some documentation or other code that explains why the existing order is wrong, and why the new order proposed here is correct?

fronald commented 3 days ago

No problem. You can close this PR. We have been using this altered version for 2 years in a high-criticality production system, and there are no issues on FreeBSD. Forgive my English, but when I referred to the order, I was talking about the "getFieldOrder" field. So, the problem is basically that on FreeBSD, the "ext" field was missing. We will continue applying this patch when we update the NuProcess version to support our clusters. Thank you, and now I apologize for the delay.