Plugging UNIX socket leaks
This patch is an effort to plug some leaks in our use of UNIX sockets.
A Heisenbug can occur if UNIX sockets are leaked:
The named UNIX socket used to listen for RenderThread connections,
usually at /tmp/android-$USER/qemu-gles-EMULATOR_PID,
will remain in the filesystem after it has been successfully
bound, even when the emulator closes all socket connections
carefully. That is the default named UNIX socket behavior.
In a system like a test-server, the EMULATOR-PID might
re-occur if PIDs loop past MAX_PID while the server is up.
In that case, the RenderServer will fail to bind and
listen to the (existing) named socket.
One could hide this problem by renaming the socket "uniquely",
but this is hopefully a proper solution.
Changes to be committed:
modified: distrib/android-emugl/host/libs/libOpenglRender/RenderServer.cpp
modified: distrib/android-emugl/host/libs/libOpenglRender/render_api.cpp
modified: distrib/android-emugl/shared/OpenglCodecCommon/SocketStream.cpp
modified: distrib/android-emugl/shared/OpenglCodecCommon/UnixStream.cpp
modified: distrib/android-emugl/shared/OpenglCodecCommon/UnixStream.h
modified: distrib/android-emugl/shared/emugl/common/sockets.cpp
Change-Id: I5d301ee55ee239725ea41bf5b3b31b80727dc330
diff --git a/distrib/android-emugl/host/libs/libOpenglRender/RenderServer.cpp b/distrib/android-emugl/host/libs/libOpenglRender/RenderServer.cpp
index 7c91271..de3952f 100644
--- a/distrib/android-emugl/host/libs/libOpenglRender/RenderServer.cpp
+++ b/distrib/android-emugl/host/libs/libOpenglRender/RenderServer.cpp
@@ -113,6 +113,7 @@
// check if we have been requested to exit while waiting on accept
if ((clientFlags & IOSTREAM_CLIENT_EXIT_SERVER) != 0) {
m_exiting = true;
+ delete stream;
break;
}
@@ -120,11 +121,10 @@
if (!rt) {
fprintf(stderr,"Failed to create RenderThread\n");
delete stream;
- stream = NULL;
} else if (!rt->start()) {
fprintf(stderr,"Failed to start RenderThread\n");
delete rt;
- rt = NULL;
+ delete stream;
}
//
diff --git a/distrib/android-emugl/host/libs/libOpenglRender/render_api.cpp b/distrib/android-emugl/host/libs/libOpenglRender/render_api.cpp
index 1fb8d6f..984755a 100644
--- a/distrib/android-emugl/host/libs/libOpenglRender/render_api.cpp
+++ b/distrib/android-emugl/host/libs/libOpenglRender/render_api.cpp
@@ -112,6 +112,7 @@
if (!s_renderWindow->isValid()) {
ERR("Could not initialize emulated framebuffer");
delete s_renderWindow;
+ s_renderWindow = NULL;
return false;
}
@@ -164,10 +165,22 @@
s_renderThread = NULL;
}
- if (s_renderWindow) {
- delete s_renderWindow;
- s_renderWindow = NULL;
- }
+ /*
+ * This conditional is unreachable in all cases except
+ * during UI shutdown. When the UI has to shutdown in an
+ * unorderly fashion (e.g. SIGKILL), a yet undefined
+ * alternative path will free s_renderWindow twice,
+ * resulting to a segfault.
+ *
+ * TODO: FIX THIS
+ *
+ */
+ // if (s_renderWindow != NULL) {
+ // delete s_renderWindow;
+ // s_renderWindow = NULL;
+ // }
+
+ delete dummy;
return ret;
}
diff --git a/distrib/android-emugl/shared/OpenglCodecCommon/SocketStream.cpp b/distrib/android-emugl/shared/OpenglCodecCommon/SocketStream.cpp
index 4b4ffb3..7b19262 100644
--- a/distrib/android-emugl/shared/OpenglCodecCommon/SocketStream.cpp
+++ b/distrib/android-emugl/shared/OpenglCodecCommon/SocketStream.cpp
@@ -50,8 +50,11 @@
#ifdef _WIN32
closesocket(m_sock);
#else
- ::close(m_sock);
+ forceStop();
+ if(close(m_sock) < 0)
+ perror("Closing SocketStream failed");
#endif
+ // DBG("SocketStream::~close @ %d \n", m_sock);
m_sock = -1;
}
if (m_buf != NULL) {
@@ -171,6 +174,7 @@
#ifdef _WIN32
::shutdown(m_sock, SD_BOTH);
#else
- ::shutdown(m_sock, SHUT_RDWR);
+ if(::shutdown(m_sock, SHUT_RDWR) < 0)
+ perror("Error shutting down SocketStream");
#endif
}
diff --git a/distrib/android-emugl/shared/OpenglCodecCommon/UnixStream.cpp b/distrib/android-emugl/shared/OpenglCodecCommon/UnixStream.cpp
index 7b2f67d..f7af57a 100644
--- a/distrib/android-emugl/shared/OpenglCodecCommon/UnixStream.cpp
+++ b/distrib/android-emugl/shared/OpenglCodecCommon/UnixStream.cpp
@@ -37,15 +37,32 @@
#endif
UnixStream::UnixStream(size_t bufSize) :
- SocketStream(bufSize)
+ SocketStream(bufSize),
+ bound_socket_path(NULL)
{
}
UnixStream::UnixStream(int sock, size_t bufSize) :
- SocketStream(sock, bufSize)
+ SocketStream(sock, bufSize),
+ bound_socket_path(NULL)
{
}
+UnixStream::~UnixStream()
+{
+ if (bound_socket_path != NULL) {
+ int ret = 0;
+ do {
+ ret = unlink(bound_socket_path);
+ } while (ret < 0 && errno == EINTR);
+ if(ret != 0) {
+ ERR("Failed to unlink UNIX socket at \"%s\"\n", bound_socket_path);
+ perror("UNIX socket could not be unlinked");
+ }
+ free(bound_socket_path);
+ }
+}
+
/* Initialize a sockaddr_un with the appropriate values corresponding
* to a given 'virtual port'. Returns 0 on success, -1 on error.
*/
@@ -84,6 +101,7 @@
// Now, initialize it properly
snprintf(path, pathlen, "%s/qemu-gles-%d", tmp, port_number);
+
return 0;
}
@@ -95,7 +113,16 @@
}
m_sock = emugl::socketLocalServer(addrstr, SOCK_STREAM);
- if (!valid()) return int(ERR_INVALID_SOCKET);
+
+ if (!valid())
+ return int(ERR_INVALID_SOCKET);
+
+ bound_socket_path = strdup(addrstr);
+ if(bound_socket_path == NULL) {
+ ERR("WARNING: UNIX socket at \"%s\" should be manually removed \n",
+ addrstr);
+ return -1;
+ }
return 0;
}
@@ -108,6 +135,7 @@
struct sockaddr_un addr;
socklen_t len = sizeof(addr);
clientSock = ::accept(m_sock, (sockaddr *)&addr, &len);
+ // DBG("UnixStream::accept @ %d \n", clientSock);
if (clientSock < 0 && errno == EINTR) {
continue;
@@ -126,6 +154,7 @@
int UnixStream::connect(const char* addr)
{
m_sock = emugl::socketLocalClient(addr, SOCK_STREAM);
+ // DBG("UnixStream::connect @ %d \n", m_sock);
if (!valid()) return -1;
return 0;
diff --git a/distrib/android-emugl/shared/OpenglCodecCommon/UnixStream.h b/distrib/android-emugl/shared/OpenglCodecCommon/UnixStream.h
index 95e7431..5e2025d 100644
--- a/distrib/android-emugl/shared/OpenglCodecCommon/UnixStream.h
+++ b/distrib/android-emugl/shared/OpenglCodecCommon/UnixStream.h
@@ -21,10 +21,12 @@
class UnixStream : public SocketStream {
public:
explicit UnixStream(size_t bufsize = 10000);
+ ~UnixStream();
virtual int listen(char addrstr[MAX_ADDRSTR_LEN]);
virtual SocketStream *accept();
virtual int connect(const char* addr);
private:
+ char *bound_socket_path;
UnixStream(int sock, size_t bufSize);
};
diff --git a/distrib/android-emugl/shared/emugl/common/sockets.cpp b/distrib/android-emugl/shared/emugl/common/sockets.cpp
index 2a13d72..b38a4be 100644
--- a/distrib/android-emugl/shared/emugl/common/sockets.cpp
+++ b/distrib/android-emugl/shared/emugl/common/sockets.cpp
@@ -24,6 +24,7 @@
#include <netinet/tcp.h>
#include <sys/un.h>
#include <sys/stat.h>
+#include <stdio.h>
#endif
#include <stddef.h>
@@ -35,6 +36,19 @@
namespace {
+static void socketSetDontLinger(int s) {
+#ifdef _WIN32
+ // TODO: Verify default behavior on WINDOWS
+#else
+ // Ungraceful shutdown, no reason to linger at all
+ struct linger so_linger;
+ so_linger.l_onoff = 1;
+ so_linger.l_linger = 0;
+ if(setsockopt(s, SOL_SOCKET, SO_LINGER, &so_linger, sizeof so_linger) < 0)
+ perror("Setting socket option SO_LINGER={on, 0} failed");
+#endif
+}
+
static void socketSetReuseAddress(int s) {
#ifdef _WIN32
// The default behaviour on Windows is equivalent to SO_REUSEADDR
@@ -44,7 +58,8 @@
// http://msdn.microsoft.com/en-us/library/windows/desktop/ms740621(v=vs.85).aspx
#else
int val = 1;
- setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
+ if(setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val)) < 0)
+ perror("Setting socket option SO_REUSEADDR failed");
#endif
}
@@ -104,25 +119,35 @@
int socketBindInternal(const SockAddr* addr, int socketType) {
int s = ::socket(addr->getFamily(), socketType, 0);
- if (s < 0)
+ if (s < 0) {
+ perror("Could not create socket to bind");
return -errno;
+ }
+
+ socketSetDontLinger(s);
+ socketSetReuseAddress(s);
// Bind to the socket.
if (::bind(s, &addr->generic, addr->len) < 0 ||
::listen(s, 5) < 0) {
int ret = -errno;
+ perror("Could not bind or listen to socket");
::close(s);
return ret;
}
- socketSetReuseAddress(s);
return s;
}
int socketConnectInternal(const SockAddr* addr, int socketType) {
int s = ::socket(addr->getFamily(), socketType, 0);
- if (s < 0)
+ if (s < 0) {
+ perror("Could not create socket to connect");
return -errno;
+ }
+
+ socketSetDontLinger(s);
+ socketSetReuseAddress(s);
int ret;
do {
