Commit ad4a9ff7 authored by Philip Pronin's avatar Philip Pronin Committed by Andrew Cox

fix AsyncServerSocket::bind issue

Summary:
When closing sockets on retry, we should use `shutdownSocketSet_`
if it is set (so socket will properly be removed from the set).

Also I changed all `::close()` and `::shutdown()` calls to `*NoInt()`
version.

Test Plan:
fbconfig unicorn/test:basic_compression_test && fbmake opt -j32
while _bin/unicorn/test/test:basic_compression_test; do echo DONE; done

Reviewed By: mcduff@fb.com

Subscribers: trunkagent, folly-diffs@, yzhan, yfeldblum

FB internal diff: D1821104

Tasks: 4752579, 6123510

Signature: t1:1821104:1423001050:5bc09ffdd6666c2884ea82dbd70831a56513cfc9

Blame Revision: D1795120
parent d8386e34
...@@ -20,17 +20,18 @@ ...@@ -20,17 +20,18 @@
#include <folly/io/async/AsyncServerSocket.h> #include <folly/io/async/AsyncServerSocket.h>
#include <folly/FileUtil.h>
#include <folly/SocketAddress.h>
#include <folly/io/async/EventBase.h> #include <folly/io/async/EventBase.h>
#include <folly/io/async/NotificationQueue.h> #include <folly/io/async/NotificationQueue.h>
#include <folly/SocketAddress.h>
#include <errno.h> #include <errno.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h> #include <fcntl.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/tcp.h> #include <netinet/tcp.h>
#include <string.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <unistd.h>
namespace folly { namespace folly {
...@@ -185,10 +186,10 @@ int AsyncServerSocket::stopAccepting(int shutdownFlags) { ...@@ -185,10 +186,10 @@ int AsyncServerSocket::stopAccepting(int shutdownFlags) {
if (shutdownSocketSet_) { if (shutdownSocketSet_) {
shutdownSocketSet_->close(handler.socket_); shutdownSocketSet_->close(handler.socket_);
} else if (shutdownFlags >= 0) { } else if (shutdownFlags >= 0) {
result = ::shutdown(handler.socket_, shutdownFlags); result = shutdownNoInt(handler.socket_, shutdownFlags);
pendingCloseSockets_.push_back(handler.socket_); pendingCloseSockets_.push_back(handler.socket_);
} else { } else {
::close(handler.socket_); closeNoInt(handler.socket_);
} }
} }
sockets_.clear(); sockets_.clear();
...@@ -216,8 +217,8 @@ int AsyncServerSocket::stopAccepting(int shutdownFlags) { ...@@ -216,8 +217,8 @@ int AsyncServerSocket::stopAccepting(int shutdownFlags) {
void AsyncServerSocket::destroy() { void AsyncServerSocket::destroy() {
stopAccepting(); stopAccepting();
for (auto s: pendingCloseSockets_) { for (auto s : pendingCloseSockets_) {
::close(s); closeNoInt(s);
} }
// Then call DelayedDestruction::destroy() to take care of // Then call DelayedDestruction::destroy() to take care of
// whether or not we need immediate or delayed destruction // whether or not we need immediate or delayed destruction
...@@ -282,7 +283,7 @@ void AsyncServerSocket::bindSocket( ...@@ -282,7 +283,7 @@ void AsyncServerSocket::bindSocket(
sockaddr* saddr = reinterpret_cast<sockaddr*>(&addrStorage); sockaddr* saddr = reinterpret_cast<sockaddr*>(&addrStorage);
if (::bind(fd, saddr, address.getActualSize()) != 0) { if (::bind(fd, saddr, address.getActualSize()) != 0) {
if (!isExistingSocket) { if (!isExistingSocket) {
::close(fd); closeNoInt(fd);
} }
folly::throwSystemError(errno, folly::throwSystemError(errno,
"failed to bind to async server socket: " + "failed to bind to async server socket: " +
...@@ -360,9 +361,7 @@ void AsyncServerSocket::bind(uint16_t port) { ...@@ -360,9 +361,7 @@ void AsyncServerSocket::bind(uint16_t port) {
"bad getaddrinfo"); "bad getaddrinfo");
} }
folly::ScopeGuard guard = folly::makeGuard([&]{ SCOPE_EXIT { freeaddrinfo(res0); };
freeaddrinfo(res0);
});
auto setupAddress = [&] (struct addrinfo* res) { auto setupAddress = [&] (struct addrinfo* res) {
int s = socket(res->ai_family, res->ai_socktype, res->ai_protocol); int s = socket(res->ai_family, res->ai_socktype, res->ai_protocol);
...@@ -375,7 +374,7 @@ void AsyncServerSocket::bind(uint16_t port) { ...@@ -375,7 +374,7 @@ void AsyncServerSocket::bind(uint16_t port) {
try { try {
setupSocket(s); setupSocket(s);
} catch (...) { } catch (...) {
::close(s); closeNoInt(s);
throw; throw;
} }
...@@ -399,6 +398,7 @@ void AsyncServerSocket::bind(uint16_t port) { ...@@ -399,6 +398,7 @@ void AsyncServerSocket::bind(uint16_t port) {
} }
}; };
const int kNumTries = 5;
for (int tries = 1; true; tries++) { for (int tries = 1; true; tries++) {
// Prefer AF_INET6 addresses. RFC 3484 mandates that getaddrinfo // Prefer AF_INET6 addresses. RFC 3484 mandates that getaddrinfo
// should return IPv6 first and then IPv4 addresses, but glibc's // should return IPv6 first and then IPv4 addresses, but glibc's
...@@ -413,8 +413,9 @@ void AsyncServerSocket::bind(uint16_t port) { ...@@ -413,8 +413,9 @@ void AsyncServerSocket::bind(uint16_t port) {
} }
// If port == 0, then we should try to bind to the same port on ipv4 and // If port == 0, then we should try to bind to the same port on ipv4 and
// ipv6. So if we did bind to ipv6, figure out that port and use it. // ipv6. So if we did bind to ipv6, figure out that port and use it,
if (!sockets_.empty() && port == 0) { // except for the last attempt when we just use any port available.
if (sockets_.size() == 1 && port == 0) {
SocketAddress address; SocketAddress address;
address.setFromLocalAddress(sockets_.back().socket_); address.setFromLocalAddress(sockets_.back().socket_);
snprintf(sport, sizeof(sport), "%u", address.getPort()); snprintf(sport, sizeof(sport), "%u", address.getPort());
...@@ -425,17 +426,21 @@ void AsyncServerSocket::bind(uint16_t port) { ...@@ -425,17 +426,21 @@ void AsyncServerSocket::bind(uint16_t port) {
try { try {
for (res = res0; res; res = res->ai_next) { for (res = res0; res; res = res->ai_next) {
if (res->ai_family != AF_INET6) { if (res->ai_family != AF_INET6) {
setupAddress(res); setupAddress(res);
} }
} }
} catch (const std::system_error& e) { } catch (const std::system_error& e) {
// if we can't bind to the same port on ipv4 as ipv6 when using port=0 // if we can't bind to the same port on ipv4 as ipv6 when using port=0
// then we will try again another 2 times before giving up. We do this // then we will try again another 2 times before giving up. We do this
// by closing the sockets that were opened, then redoing the whole thing // by closing the sockets that were opened, then redoing the whole thing
if (port == 0 && !sockets_.empty() && tries != 3) { if (port == 0 && !sockets_.empty() && tries != kNumTries) {
for (const auto& socket : sockets_) { for (const auto& socket : sockets_) {
if (socket.socket_ > 0) { if (socket.socket_ <= 0) {
CHECK_EQ(0, ::close(socket.socket_)); continue;
} else if (shutdownSocketSet_) {
shutdownSocketSet_->close(socket.socket_);
} else {
closeNoInt(socket.socket_);
} }
} }
sockets_.clear(); sockets_.clear();
...@@ -625,7 +630,7 @@ int AsyncServerSocket::createSocket(int family) { ...@@ -625,7 +630,7 @@ int AsyncServerSocket::createSocket(int family) {
try { try {
setupSocket(fd); setupSocket(fd);
} catch (...) { } catch (...) {
::close(fd); closeNoInt(fd);
throw; throw;
} }
return fd; return fd;
...@@ -735,7 +740,7 @@ void AsyncServerSocket::handlerReady( ...@@ -735,7 +740,7 @@ void AsyncServerSocket::handlerReady(
} else if (rand() > acceptRate_ * RAND_MAX) { } else if (rand() > acceptRate_ * RAND_MAX) {
++numDroppedConnections_; ++numDroppedConnections_;
if (clientSocket >= 0) { if (clientSocket >= 0) {
::close(clientSocket); closeNoInt(clientSocket);
} }
continue; continue;
} }
...@@ -765,7 +770,7 @@ void AsyncServerSocket::handlerReady( ...@@ -765,7 +770,7 @@ void AsyncServerSocket::handlerReady(
#ifndef SOCK_NONBLOCK #ifndef SOCK_NONBLOCK
// Explicitly set the new connection to non-blocking mode // Explicitly set the new connection to non-blocking mode
if (fcntl(clientSocket, F_SETFL, O_NONBLOCK) != 0) { if (fcntl(clientSocket, F_SETFL, O_NONBLOCK) != 0) {
::close(clientSocket); closeNoInt(clientSocket);
dispatchError("failed to set accepted socket to non-blocking mode", dispatchError("failed to set accepted socket to non-blocking mode",
errno); errno);
return; return;
...@@ -829,7 +834,7 @@ void AsyncServerSocket::dispatchSocket(int socket, ...@@ -829,7 +834,7 @@ void AsyncServerSocket::dispatchSocket(int socket,
// even accept new messages. // even accept new messages.
LOG(ERROR) << "failed to dispatch newly accepted socket:" LOG(ERROR) << "failed to dispatch newly accepted socket:"
<< " all accept callback queues are full"; << " all accept callback queues are full";
::close(socket); closeNoInt(socket);
return; return;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment