Fixes hangs if Wine is built with newer headers than kernel
-- v2: server: Fall back to epoll_wait if epoll_pwait2 doesn't work.
From: Alfred Agrell floating@muncher.se
Fixes: 87ca5db40e2c1b37423bdc25101a5c5e39e67e6f --- server/fd.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/server/fd.c b/server/fd.c index e1b969a0439..bb0e90d99d0 100644 --- a/server/fd.c +++ b/server/fd.c @@ -566,6 +566,9 @@ static inline void main_loop_epoll(void) int i, ret, timeout; struct timespec ts; struct epoll_event events[128]; +#ifdef HAVE_EPOLL_PWAIT2 + static int failed_epoll_pwait2 = 0; +#endif
assert( POLLIN == EPOLLIN ); assert( POLLOUT == EPOLLOUT ); @@ -582,10 +585,15 @@ static inline void main_loop_epoll(void) if (epoll_fd == -1) break; /* an error occurred with epoll */
#ifdef HAVE_EPOLL_PWAIT2 - ret = epoll_pwait2( epoll_fd, events, ARRAY_SIZE( events ), timeout == -1 ? NULL : &ts, NULL ); -#else - ret = epoll_wait( epoll_fd, events, ARRAY_SIZE( events ), timeout ); + if (!failed_epoll_pwait2) + { + ret = epoll_pwait2( epoll_fd, events, ARRAY_SIZE( events ), timeout == -1 ? NULL : &ts, NULL ); + if (ret == -1 && errno == ENOSYS) + failed_epoll_pwait2 = 1; + } + if (failed_epoll_pwait2) #endif + ret = epoll_wait( epoll_fd, events, ARRAY_SIZE( events ), timeout );
set_current_time();
I was aware of this myself and considered caring about it, but ultimately decided that it fell into the category of an "unsupported configuration".
As an extreme example of an unsupported configuration, a Wine built on Ubuntu 24.04 would be broken in quite a few other ways (glibc, for one) if you tried running it on Ubuntu 18.04. The same source compiled on Ubuntu 18.04 (or earlier) would work fine, OTOH.
I just didn't see why this example should be special cased, so I didn't put any effort into justifying something like this myself.
Despite that, I'm not against this patch specifically at all, it looks like it should do the trick. I'll just attach two versions of something similar I made, for posterity.
[weaksym.patch](/uploads/3ce6d9e2a7612ce93610621a6bdad6dc/weaksym.patch) [dlsym.patch](/uploads/5d4001fabc5f8fe94c01a8091a170653/dlsym.patch)
On Sat Mar 22 11:46:55 2025 +0000, William Horvath wrote:
I was aware of this myself and considered caring about it, but ultimately decided that it fell into the category of an "unsupported configuration". As an extreme example of an unsupported configuration, a Wine built on Ubuntu 24.04 would be broken in quite a few other ways (glibc, for one) if you tried running it on Ubuntu 18.04. The same source compiled on Ubuntu 18.04 (or earlier) would work fine, OTOH. I just didn't see why this example should be special cased, so I didn't put any effort into justifying something like this myself. Despite that, I'm not against this patch specifically at all, it looks like it should do the trick. I'll just attach two versions of something similar I made, for posterity. [weaksym.patch](/uploads/3ce6d9e2a7612ce93610621a6bdad6dc/weaksym.patch) [dlsym.patch](/uploads/5d4001fabc5f8fe94c01a8091a170653/dlsym.patch)
Compiling against some library and running against an older version is not supported, yes.
But I'd say the kernel should be a special case. Some distros support multiple kernel versions; compiling on one distro and running on the same should be supported, even if you're choosing an older-than-default kernel. Some guy on winehackers IRC did exactly that and ran into this issue.
(Your patches solve a slightly different issue - they check if epoll_pwait2 is present at runtime, but if it is, they assume it works and won't return ENOSYS.)
On Sat Mar 22 13:04:43 2025 +0000, Alfred Agrell wrote:
Compiling against some library and running against an older version is not supported, yes. But I'd say the kernel should be a special case. Some distros support multiple kernel versions; compiling on one distro and running on the same should be supported, even if you're choosing an older-than-default kernel. Some guy on winehackers IRC did exactly that and ran into this issue. (Your patches solve a slightly different issue - they check if epoll_pwait2 is present at runtime, but if it is, they assume it works and won't return ENOSYS.)
Actually, epoll_pwait2 simply won't be resolved if it's not supported. We don't care about the return value if we never attempt to run it in the first place.
I still disagree with the idea in principle, though. Where should we draw the line? Should Wine built against 6.14 headers work on 5.10? I don't think this has ever been supported, outside of third party libraries which are explicitly loaded by dlopen().
Anyways, if julliard agrees that this is one specific case to support for backwards compatibility, then so be it.
(I'll resolve the thread because I don't actually have a concrete argument against this specific patch, only an opinion for the general case.)
On Sat Mar 22 15:06:10 2025 +0000, William Horvath wrote:
Actually, epoll_pwait2 simply won't be resolved if it's not supported. We don't care about the return value if we never attempt to run it in the first place. I still disagree with the idea in principle, though. Where should we draw the line? Should Wine built against 6.14 headers work on 5.10? I don't think this has ever been supported, outside of third party libraries which are explicitly loaded by dlopen(). Anyways, if julliard agrees that this is one specific case to support for backwards compatibility, then so be it. (I'll resolve the thread because I don't actually have a concrete argument against this specific patch, only an opinion for the general case.)
Yes, in general we want to support older kernels, the libc version is not tied to the kernel version.
Perhaps not strictly relevant to this MR, but the system does not support `epoll_wait2`, would it be worth the effort to use `timerfd` to achieve nanosecond resolution with `epoll_wait` instead? Timerfd has been there since 2.6.25, so it can be used on older systems missing `epoll_wait`.
-
Alexandre Julliard (＠julliard) -
Alfred Agrell
-
Alfred Agrell (＠Alcaro)
-
Vibhav Pant (＠vibhavp)
-
William Horvath (＠whrvt)