reactphp / socket

Async, streaming plaintext TCP/IP and secure TLS socket server and client connections for ReactPHP.
https://reactphp.org/socket/
MIT License
1.2k stars 156 forks source link

Avoid unneeded syscall on socket close #292

Closed clue closed 2 years ago

clue commented 2 years ago

This simple PR removes an unneeded syscall on socket close. In this state, the socket is already in non-blocking mode anyway and about to be closed anyway, so setting non-blocking mode again provides very little value whatsoever. The comment above this command suggests this might have been needed for some (unknown) legacy version, but careful testing and our test suite confirms this behavior can not be exhibited on any known versions. Given the socket is closed immediately after this call, there's little chance this could cause unwanted side effects in any real world applications in either case.

This change does not affect performance in any noticeable way, but helps avoid unneeded syscalls in an attempt to ease debugging and improve overall performance (see https://github.com/reactphp/http/issues/455). For a single HTTP request, this means the following syscalls can be eliminated:

pselect6(5, [4], [], [], NULL, NULL)    = 1 (in [4])
poll([{fd=4, events=POLLIN|POLLERR|POLLHUP}], 1, 0) = 1 ([{fd=4, revents=POLLIN}])
accept(4, {sa_family=AF_INET6, sin6_port=htons(54462), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:127.0.0.1", &sin6_addr), sin6_scope_id=0}, [128 => 28]) = 3
fcntl(3, F_GETFL)                       = 0x2 (flags O_RDWR)
fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK)    = 0
fcntl(3, F_GETFL)                       = 0x802 (flags O_RDWR|O_NONBLOCK)
fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK)    = 0
pselect6(5, [3 4], [], [], NULL, NULL)  = 1 (in [3])
poll([{fd=3, events=POLLIN|POLLPRI|POLLERR|POLLHUP}], 1, 0) = 1 ([{fd=3, revents=POLLIN}])
recvfrom(3, "G", 1, MSG_PEEK, NULL, NULL) = 1
recvfrom(3, "GET / HTTP/1.1\r\nHost: localhost:"..., 65536, 0, NULL, NULL) = 78
poll([{fd=3, events=POLLIN|POLLPRI|POLLERR|POLLHUP}], 1, 0) = 0 (Timeout)
recvfrom(3, 0x7fceeb63d066, 65458, 0, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
getpeername(3, {sa_family=AF_INET6, sin6_port=htons(54462), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:127.0.0.1", &sin6_addr), sin6_scope_id=0}, [128 => 28]) = 0
getsockname(3, {sa_family=AF_INET6, sin6_port=htons(8080), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:127.0.0.1", &sin6_addr), sin6_scope_id=0}, [128 => 28]) = 0
gettimeofday({tv_sec=1652528350, tv_usec=915571}, NULL) = 0
gettimeofday({tv_sec=1652528350, tv_usec=915621}, NULL) = 0
gettimeofday({tv_sec=1652528350, tv_usec=915765}, NULL) = 0
pselect6(5, [3 4], [3], [], NULL, NULL) = 1 (out [3])
sendto(3, "HTTP/1.1 200 OK\r\nContent-Type: t"..., 150, 0, NULL, 0) = 150
pselect6(5, [3 4], [], [], NULL, NULL)  = 1 (in [3])
poll([{fd=3, events=POLLIN|POLLPRI|POLLERR|POLLHUP}], 1, 0) = 1 ([{fd=3, revents=POLLIN}])
recvfrom(3, "", 1, MSG_PEEK, NULL, NULL) = 0
recvfrom(3, "", 65536, 0, NULL, NULL)   = 0
shutdown(3, SHUT_RDWR)                  = 0
- fcntl(3, F_GETFL)                       = 0x802 (flags O_RDWR|O_NONBLOCK)
- fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK)    = 0
close(3)                                = 0

pselect6(5, [4], [], [], NULL, NULL)    = …

This changeset only removes some internal logic and does not affect the public API, so it should be safe to apply. The test suite confirms this has 100% code coverage.

Refs https://github.com/reactphp/http/issues/455