Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linux: epoll_pwait() syscall wrapper is probably wrong #4

Closed
bnoordhuis opened this issue Nov 24, 2014 · 6 comments
Closed

linux: epoll_pwait() syscall wrapper is probably wrong #4

bnoordhuis opened this issue Nov 24, 2014 · 6 comments
Labels

Comments

@bnoordhuis
Copy link
Member

It currently passes sizeof(sigset_t) to the kernel but that's wrong on at least x86_64. A patch:

diff --git a/src/unix/linux-syscalls.c b/src/unix/linux-syscalls.c
index 06cc594..ee90b84 100644
--- a/src/unix/linux-syscalls.c
+++ b/src/unix/linux-syscalls.c
@@ -21,6 +21,7 @@

 #include "linux-syscalls.h"
 #include <unistd.h>
+#include <signal.h>
 #include <sys/syscall.h>
 #include <sys/types.h>
 #include <errno.h>
@@ -298,7 +299,7 @@ int uv__epoll_pwait(int epfd,
                  nevents,
                  timeout,
                  sigmask,
-                 sizeof(*sigmask));
+                 _NSIG / 8);
 #else
   return errno = ENOSYS, -1;
 #endif

_NSIG / 8 is 8, whereas sizeof(*sigmask) is 128 and makes the system call fail with EINVAL. I'm reasonably sure that the above is correct (it's how glibc implements epoll_wait) but it perhaps needs some additional verification.

@bnoordhuis bnoordhuis added the bug label Nov 24, 2014
bnoordhuis added a commit to bnoordhuis/node that referenced this issue Nov 24, 2014
* Block SIGPROF when in the epoll_pwait() system call so the event loop
  doesn't keep waking up on signal delivery.  The clock_gettime() system
  call that libuv does after EINTR is very expensive on virtualized
  systems.

* Replace sched_yield() with nanosleep() in V8's tick event processor
  thread.  The former only yields the CPU when there is another process
  scheduled on the same CPU.

* Fix a bug in the epoll_pwait() system call wrapper in libuv,
  see libuv/libuv#4.

Refs strongloop/strong-agent#3 and strongloop-internal/scrum-cs#37.
@saghul
Copy link
Member

saghul commented Nov 25, 2014

IIRC this was tested both by the reporter and by @lucab on Debian's AArch64 build: joyent/libuv#1443 any chance things should be handled differently depending on the architecture here?

@saghul
Copy link
Member

saghul commented Nov 25, 2014

Ah, wait, in libuv we always pass NULL to the call uv__epoll_pwait, that's why it never surfaced. LGTM if you are sure it works. But otherwise we can fix it later too :-)

@saghul
Copy link
Member

saghul commented Nov 25, 2014

musl libc also does it, so land it at will.

bnoordhuis added a commit that referenced this issue Nov 25, 2014
sizeof(sigset_t) = 128 whereas the kernel expects 8, the size of a long.

It made the system call fail with EINVAL when a non-NULL sigset was
passed in.  Fortunately, it's academic because there is just one call
site and it passes in NULL.

Fixes #4.
@bnoordhuis
Copy link
Member Author

Cheers, landed in v0.10 in b705b53. What's the current strategy for up-merges? At will or after a release?

@saghul
Copy link
Member

saghul commented Nov 25, 2014

Usually after a release, but if you need it feel free to up-merge.

@bnoordhuis
Copy link
Member Author

No rush, I'll wait for the next release.

@zhjzjnb zhjzjnb mentioned this issue Aug 10, 2016
erw7 pushed a commit to erw7/libuv that referenced this issue Nov 2, 2018
zhaozg added a commit to zhaozg/libuv that referenced this issue Jan 18, 2021
ERROR: LeakSanitizer: detected memory leaks

```
Direct leak of 432 byte(s) in 9 object(s) allocated from:
    #0 0x1062eedc2 in __sanitizer_mz_calloc+0x92 (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x46dc2)
    #1 0x7fff20171eb6 in _malloc_zone_calloc+0x3a (libsystem_malloc.dylib:x86_64+0x1beb6)
    libuv#2 0x7fff203ac180 in _CFRuntimeCreateInstance+0x124 (CoreFoundation:x86_64h+0x4180)
    libuv#3 0x7fff203ab906 in __CFStringCreateImmutableFunnel3+0x84d (CoreFoundation:x86_64h+0x3906)
    libuv#4 0x7fff203ab0a1 in CFStringCreateWithCString+0x48 (CoreFoundation:x86_64h+0x30a1)
    libuv#5 0x1056f63e1 in uv__get_cpu_speed darwin.c:267
    libuv#6 0x1056f491e in uv_cpu_info darwin.c:338
```
zhaozg added a commit to zhaozg/libuv that referenced this issue Jan 19, 2021
ERROR: LeakSanitizer: detected memory leaks

```
Direct leak of 432 byte(s) in 9 object(s) allocated from:
    #0 0x1062eedc2 in __sanitizer_mz_calloc+0x92 (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x46dc2)
    #1 0x7fff20171eb6 in _malloc_zone_calloc+0x3a (libsystem_malloc.dylib:x86_64+0x1beb6)
    libuv#2 0x7fff203ac180 in _CFRuntimeCreateInstance+0x124 (CoreFoundation:x86_64h+0x4180)
    libuv#3 0x7fff203ab906 in __CFStringCreateImmutableFunnel3+0x84d (CoreFoundation:x86_64h+0x3906)
    libuv#4 0x7fff203ab0a1 in CFStringCreateWithCString+0x48 (CoreFoundation:x86_64h+0x30a1)
    libuv#5 0x1056f63e1 in uv__get_cpu_speed darwin.c:267
    libuv#6 0x1056f491e in uv_cpu_info darwin.c:338
```
zhaozg added a commit to zhaozg/libuv that referenced this issue Mar 20, 2021
ERROR: LeakSanitizer: detected memory leaks

```
Direct leak of 432 byte(s) in 9 object(s) allocated from:
    #0 0x1062eedc2 in __sanitizer_mz_calloc+0x92 (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x46dc2)
    #1 0x7fff20171eb6 in _malloc_zone_calloc+0x3a (libsystem_malloc.dylib:x86_64+0x1beb6)
    libuv#2 0x7fff203ac180 in _CFRuntimeCreateInstance+0x124 (CoreFoundation:x86_64h+0x4180)
    libuv#3 0x7fff203ab906 in __CFStringCreateImmutableFunnel3+0x84d (CoreFoundation:x86_64h+0x3906)
    libuv#4 0x7fff203ab0a1 in CFStringCreateWithCString+0x48 (CoreFoundation:x86_64h+0x30a1)
    libuv#5 0x1056f63e1 in uv__get_cpu_speed darwin.c:267
    libuv#6 0x1056f491e in uv_cpu_info darwin.c:338
```
santigimeno pushed a commit that referenced this issue Apr 4, 2021
ERROR: LeakSanitizer: detected memory leaks

```
Direct leak of 432 byte(s) in 9 object(s) allocated from:
    #0 0x1062eedc2 in __sanitizer_mz_calloc+0x92 (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x46dc2)
    #1 0x7fff20171eb6 in _malloc_zone_calloc+0x3a (libsystem_malloc.dylib:x86_64+0x1beb6)
    #2 0x7fff203ac180 in _CFRuntimeCreateInstance+0x124 (CoreFoundation:x86_64h+0x4180)
    #3 0x7fff203ab906 in __CFStringCreateImmutableFunnel3+0x84d (CoreFoundation:x86_64h+0x3906)
    #4 0x7fff203ab0a1 in CFStringCreateWithCString+0x48 (CoreFoundation:x86_64h+0x30a1)
    #5 0x1056f63e1 in uv__get_cpu_speed darwin.c:267
    #6 0x1056f491e in uv_cpu_info darwin.c:338
```

PR-URL: #3098
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
JeffroMF pushed a commit to JeffroMF/libuv that referenced this issue May 16, 2022
ERROR: LeakSanitizer: detected memory leaks

```
Direct leak of 432 byte(s) in 9 object(s) allocated from:
    #0 0x1062eedc2 in __sanitizer_mz_calloc+0x92 (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x46dc2)
    #1 0x7fff20171eb6 in _malloc_zone_calloc+0x3a (libsystem_malloc.dylib:x86_64+0x1beb6)
    #2 0x7fff203ac180 in _CFRuntimeCreateInstance+0x124 (CoreFoundation:x86_64h+0x4180)
    #3 0x7fff203ab906 in __CFStringCreateImmutableFunnel3+0x84d (CoreFoundation:x86_64h+0x3906)
    libuv#4 0x7fff203ab0a1 in CFStringCreateWithCString+0x48 (CoreFoundation:x86_64h+0x30a1)
    libuv#5 0x1056f63e1 in uv__get_cpu_speed darwin.c:267
    libuv#6 0x1056f491e in uv_cpu_info darwin.c:338
```

PR-URL: libuv#3098
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants