On Mon Apr 14 13:38:03 2025 +0000, Rémi Bernon wrote:
Thanks for addressing the comments, I think it's better but I believe it would be even cleaner to refactor the code to move inproc sync / server sync behind the same interface, so that we don't have to spread out inproc sync calls a bit everywhere alongside server sync calls, while only one of the two paths is supposed to be taken. I think this will allow us to avoid diverging implementations, and to debug and bisect ntsync issues with server sync more easily, if there are any, avoiding a lot of the dead code introduction there. I have implemented this in https://gitlab.winehq.org/rbernon/wine/-/commits/wip/ntsync, and I will send the changes in separate MRs. ~~Then, on a separate note, while testing with ntsync on Arch with kernel 6.14.2-arch1-1, I have experienced some issues that I think will need to be solved before enabling it: somehow NTSYNC_IOC_CREATE_* ioctls are returning fds which have already been allocated, for instance by a dup call. This manifests with random issues and more easily spotted by running wineserver in `valgrind --track-fds=yes` which shows fds being closed twice.~~ This is probably just because valgrind doesn't know about ntsync fd-allocating functions.
Yeah, nevermind the last part, adding support for it in valgrind fixes the reported double closes. The other issue I was having was with opened file descriptor limit, which is 1024 by default and probably need to be increased (and wineserver should try to request hard limit, the same way we do for other useful limits).