uNetworking / uSockets

Miniscule cross-platform eventing, networking & crypto for async applications
Apache License 2.0
1.29k stars 267 forks source link

Are closed fd's being polled? #216

Closed markg85 closed 9 months ago

markg85 commented 9 months ago

Hi,

I just looked at the strace output of an an ab benchmark. Now there's lots of other layers here (i was running it in nodejs) so that for sure clutters the output. A thing I noticed was a lot of output like this:

epoll_pwait(13, [{events=EPOLLIN, data={u32=19, u64=19}}], 1024, 0, NULL, 8) = 1
accept4(19, {sa_family=AF_INET6, sin6_port=htons(44650), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:172.18.0.1", &sin6_addr), sin6_scope_id=0}, [128 => 28], SOCK_CLOEXEC|SOCK_NONBLOCK) = 22
fcntl(22, F_GETFL)                      = 0x802 (flags O_RDWR|O_NONBLOCK)
fcntl(22, F_SETFL, O_RDWR|O_NONBLOCK|O_LARGEFILE) = 0
epoll_ctl(13, EPOLL_CTL_ADD, 22, {events=EPOLLIN, data={u32=4294967295, u64=4294967295}}) = 0
epoll_ctl(13, EPOLL_CTL_DEL, 22, 0x7ffdcb09cd4c) = 0
ioctl(22, FIONBIO, [1])                 = 0
epoll_ctl(13, EPOLL_CTL_DEL, 22, 0x7ffdcb09cd5c) = -1 ENOENT (No such file or directory)
setsockopt(22, SOL_TCP, TCP_NODELAY, [1], 4) = 0
accept4(19, 0x7ffdcb09cde0, [128], SOCK_CLOEXEC|SOCK_NONBLOCK) = -1 EAGAIN (Resource temporarily unavailable)
epoll_ctl(13, EPOLL_CTL_ADD, 22, {events=EPOLLIN, data={u32=22, u64=22}}) = 0
epoll_pwait(13, [{events=EPOLLIN, data={u32=22, u64=22}}], 1024, 0, NULL, 8) = 1
recvfrom(22, "GET / HTTP/1.0\r\nHost: 172.18.0.1"..., 524288, 0, NULL, NULL) = 79
sendto(22, "HTTP/1.1 505 HTTP Version Not Su"..., 174, MSG_NOSIGNAL, NULL, 0) = 174
shutdown(22, SHUT_WR)                   = 0
epoll_ctl(13, EPOLL_CTL_DEL, 22, 0x7ffdcb09cdcc) = 0
epoll_ctl(13, EPOLL_CTL_DEL, 22, 0x7ffdcb09cdcc) = -1 ENOENT (No such file or directory)
close(22)                               = 0
epoll_pwait(13, [{events=EPOLLIN, data={u32=19, u64=19}}], 1024, 0, NULL, 8) = 1
accept4(19, {sa_family=AF_INET6, sin6_port=htons(44652), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:172.18.0.1", &sin6_addr), sin6_scope_id=0}, [128 => 28], SOCK_CLOEXEC|SOCK_NONBLOCK) = 22
fcntl(22, F_GETFL)                      = 0x802 (flags O_RDWR|O_NONBLOCK)
fcntl(22, F_SETFL, O_RDWR|O_NONBLOCK|O_LARGEFILE) = 0
epoll_ctl(13, EPOLL_CTL_ADD, 22, {events=EPOLLIN, data={u32=4294967295, u64=4294967295}}) = 0
epoll_ctl(13, EPOLL_CTL_DEL, 22, 0x7ffdcb09cd4c) = 0
ioctl(22, FIONBIO, [1])                 = 0
epoll_ctl(13, EPOLL_CTL_DEL, 22, 0x7ffdcb09cd5c) = -1 ENOENT (No such file or directory)
setsockopt(22, SOL_TCP, TCP_NODELAY, [1], 4) = 0
accept4(19, 0x7ffdcb09cde0, [128], SOCK_CLOEXEC|SOCK_NONBLOCK) = -1 EAGAIN (Resource temporarily unavailable)
epoll_ctl(13, EPOLL_CTL_ADD, 22, {events=EPOLLIN, data={u32=22, u64=22}}) = 0
epoll_pwait(13, [{events=EPOLLIN, data={u32=22, u64=22}}], 1024, 0, NULL, 8) = 1
recvfrom(22, "GET / HTTP/1.0\r\nHost: 172.18.0.1"..., 524288, 0, NULL, NULL) = 79
sendto(22, "HTTP/1.1 505 HTTP Version Not Su"..., 174, MSG_NOSIGNAL, NULL, 0) = 174
shutdown(22, SHUT_WR)                   = 0
epoll_ctl(13, EPOLL_CTL_DEL, 22, 0x7ffdcb09cdcc) = 0
epoll_ctl(13, EPOLL_CTL_DEL, 22, 0x7ffdcb09cdcc) = -1 ENOENT (No such file or directory)
close(22)                               = 0
epoll_pwait(13, [{events=EPOLLIN, data={u32=19, u64=19}}], 1024, 0, NULL, 8) = 1
accept4(19, {sa_family=AF_INET6, sin6_port=htons(44654), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:172.18.0.1", &sin6_addr), sin6_scope_id=0}, [128 => 28], SOCK_CLOEXEC|SOCK_NONBLOCK) = 22
fcntl(22, F_GETFL)                      = 0x802 (flags O_RDWR|O_NONBLOCK)
fcntl(22, F_SETFL, O_RDWR|O_NONBLOCK|O_LARGEFILE) = 0
epoll_ctl(13, EPOLL_CTL_ADD, 22, {events=EPOLLIN, data={u32=4294967295, u64=4294967295}}) = 0
epoll_ctl(13, EPOLL_CTL_DEL, 22, 0x7ffdcb09cd4c) = 0
ioctl(22, FIONBIO, [1])                 = 0
epoll_ctl(13, EPOLL_CTL_DEL, 22, 0x7ffdcb09cd5c) = -1 ENOENT (No such file or directory)
setsockopt(22, SOL_TCP, TCP_NODELAY, [1], 4) = 0
accept4(19, 0x7ffdcb09cde0, [128], SOCK_CLOEXEC|SOCK_NONBLOCK) = -1 EAGAIN (Resource temporarily unavailable)
epoll_ctl(13, EPOLL_CTL_ADD, 22, {events=EPOLLIN, data={u32=22, u64=22}}) = 0
epoll_pwait(13, [{events=EPOLLIN, data={u32=22, u64=22}}], 1024, 0, NULL, 8) = 1
recvfrom(22, "GET / HTTP/1.0\r\nHost: 172.18.0.1"..., 524288, 0, NULL, NULL) = 79
sendto(22, "HTTP/1.1 505 HTTP Version Not Su"..., 174, MSG_NOSIGNAL, NULL, 0) = 174
shutdown(22, SHUT_WR)                   = 0

Much of it is just stuff working. But the recurrence of:

accept4(19, 0x7ffdcb09cde0, [128], SOCK_CLOEXEC|SOCK_NONBLOCK) = -1 EAGAIN (Resource temporarily unavailable)

and

epoll_ctl(13, EPOLL_CTL_DEL, 22, 0x7ffdcb09cd5c) = -1 ENOENT (No such file or directory)

Look a bit suspicious to me.

Not sure if it's a hint for a bug somewhere or if it's intended behavior. Just wanted to report it to be sure.

Sidenote, yes, i'm aware i should not use ab, it sends http 1.0 requests.

Best regards, Mark

uNetworkingAB commented 9 months ago

EAGAIN is intended, ENOENT is not intended but probably part of libuv. You can try the same with the raw epoll and there shouldn't be any of those

uNetworkingAB commented 9 months ago

I tested with raw epoll and it is clean. With libuv I get the same issues you post here. So looks like libuv is broken.

uNetworkingAB commented 9 months ago

ENOENT op was EPOLL_CTL_MOD or EPOLL_CTL_DEL, and fd is not registered with this epoll instance.

It doesn't have to be a bug, can just be inefficient use of epoll_ctl. As long as the error is not EBADF it doesn't have to be a bug. EBADF would definitely be a critical bug.

markg85 commented 9 months ago

It doesn't have to be a bug, can just be inefficient use of epoll_ctl. As long as the error is not EBADF it doesn't have to be a bug. EBADF would definitely be a critical bug.

While that might be true, you probably know best as creator of this library. It might still be a thing affecting high performance. Your library is optimized so much for the best possible throughput that an inefficiency anywhere in the stack likely leads to a measurable performance regression.

Is there a runtime environment variable i can set to measure this without recompiling this all the way up to uWebSockets js? Just curious to know what the raw epoll version would do in tersm of req/sec.

uNetworkingAB commented 9 months ago

It's libuv, nothing we can do about it

markg85 commented 9 months ago

You ran it with raw epoll, right? How?

uNetworkingAB commented 9 months ago

Uwebsockets.js always runs with libuv because its the event loop of nodejs. The C++ lib itself defaults to raw epoll.

markg85 commented 9 months ago

Ah, that makes sense! Thank you for explaining.

Feel free to do whatever you want with this issue. The way i see it there might be a bug somewhere. But you can also argue the bug is upstream in libuv. It's up to you.