# HG changeset patch
# User jbe
# Date 1434925159 -7200
# Node ID 77dc29ce7948143b02642b8b048cdf1072d3c6a2
# Parent  2dd0eea2f5f8d7648355f659941474ab1f5e6c4e
Ensure that socket handles always get closed, even in case of an error while creating the socket handle

diff -r 2dd0eea2f5f8 -r 77dc29ce7948 moonbridge_io.c
--- a/moonbridge_io.c	Sun Jun 21 23:38:53 2015 +0200
+++ b/moonbridge_io.c	Mon Jun 22 00:19:19 2015 +0200
@@ -79,9 +79,6 @@
 
 typedef struct {
   pid_t pid;
-  int uninitialized_in;
-  int uninitialized_out;
-  int uninitialized_err;
 } moonbr_io_child_t;
 
 static int moonbr_io_yield(lua_State *L) {
@@ -742,14 +739,16 @@
   }
 }
 
-void moonbr_io_pushhandle(lua_State *L, int fd) {
+static int moonbr_io_pushhandle_impl(lua_State *L) {
+  int *fd;
   moonbr_io_handle_t *handle;
   struct sockaddr addr;
   socklen_t addrlen;
+  fd = lua_touserdata(L, 1);
   handle = lua_newuserdata(L, sizeof(moonbr_io_handle_t));
-  handle->fd = fd;
+  handle->fd = -1;  /* avoid closing incomplete handle */
   addrlen = sizeof(addr);
-  if (getsockname(fd, &addr, &addrlen)) {
+  if (getsockname(*fd, &addr, &addrlen)) {
     if (errno != ENOTSOCK) {
       moonbr_io_errmsg();
       luaL_error(L, "Unexpected error when examining socket: %s", errmsg);
@@ -774,7 +773,9 @@
   handle->writeqoff = 0;
   handle->writebufin = 0;
   handle->writebufout = 0;
+  handle->fd = *fd;  /* required for set_linger call */
   moonbr_io_handle_set_linger(L, handle, 0);
+  handle->fd = -1;  /* avoid closing incomplete handle */
   luaL_getmetatable(L, MOONBR_IO_HANDLE_MT_REGKEY);
   lua_setmetatable(L, -2);
   lua_newtable(L);  // uservalue
@@ -786,7 +787,7 @@
     char addrstrbuf[INET6_ADDRSTRLEN];
     const char *addrstr;
     addrlen = sizeof(addr_in6);
-    if (getsockname(fd, (struct sockaddr *)&addr_in6, &addrlen)) {
+    if (getsockname(*fd, (struct sockaddr *)&addr_in6, &addrlen)) {
       moonbr_io_errmsg();
       luaL_error(L, "Could not determine local IP address/port: %s", errmsg);
     }
@@ -803,7 +804,7 @@
     }
     lua_pushinteger(L, ntohs(addr_in6.sin6_port));
     lua_setfield(L, -2, "local_tcpport");
-    if (getpeername(fd, (struct sockaddr *)&addr_in6, &addrlen)) {
+    if (getpeername(*fd, (struct sockaddr *)&addr_in6, &addrlen)) {
       moonbr_io_errmsg();
       luaL_error(L, "Could not determine remote IP address/port: %s", errmsg);
     }
@@ -825,7 +826,7 @@
     char addrstrbuf[INET_ADDRSTRLEN];
     const char *addrstr;
     addrlen = sizeof(addr_in);
-    if (getsockname(fd, (struct sockaddr *)&addr_in, &addrlen)) {
+    if (getsockname(*fd, (struct sockaddr *)&addr_in, &addrlen)) {
       moonbr_io_errmsg();
       luaL_error(L, "Could not determine local IP address/port: %s", errmsg);
     }
@@ -842,7 +843,7 @@
     }
     lua_pushinteger(L, ntohs(addr_in.sin_port));
     lua_setfield(L, -2, "local_tcpport");
-    if (getpeername(fd, (struct sockaddr *)&addr_in, &addrlen)) {
+    if (getpeername(*fd, (struct sockaddr *)&addr_in, &addrlen)) {
       moonbr_io_errmsg();
       luaL_error(L, "Could not determine remote IP address/port: %s", errmsg);
     }
@@ -864,6 +865,18 @@
   lua_setmetatable(L, -2);
   lua_setfield(L, -2, "public");
   lua_setuservalue(L, -2);
+  handle->fd = *fd;
+  *fd = -1;  /* closing is now handled by garbage collection */
+  return 1;
+}
+
+void moonbr_io_pushhandle(lua_State *L, int fd) {
+  lua_pushcfunction(L, moonbr_io_pushhandle_impl);
+  lua_pushlightuserdata(L, &fd);
+  if (lua_pcall(L, 1, 1, 0)) {
+    if (fd != -1) close(fd);
+    lua_error(L);
+  }
 }
 
 static int moonbr_io_handleindex(lua_State *L) {
@@ -1242,6 +1255,12 @@
   argv = lua_newuserdata(L, (argc + 1) * sizeof(char *));
   for (i=0; i<argc; i++) argv[i] = (char *)luaL_checkstring(L, i+1);
   argv[argc] = NULL;
+  child = lua_newuserdata(L, sizeof(moonbr_io_child_t));
+  child->pid = 0;
+  lua_newtable(L);
+  lua_setuservalue(L, -2);
+  luaL_getmetatable(L, MOONBR_IO_CHILD_MT_REGKEY);
+  lua_setmetatable(L, -2);
   if (socketpair(PF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, sockin)) {
     moonbr_io_errmsg();
     lua_pushnil(L);
@@ -1266,7 +1285,6 @@
     lua_pushfstring(L, "Could not create socket pair: %s", errmsg);
     return 2;
   }
-  child = lua_newuserdata(L, sizeof(moonbr_io_child_t));
   child->pid = vfork();
   if (child->pid == -1) {
     moonbr_io_errmsg();
@@ -1281,19 +1299,19 @@
     return 2;
   }
   if (!child->pid) {
-    if (dup2(sockin[1], 0) == -1) goto moonbr_io_exec_error;
-    if (dup2(sockout[1], 1) == -1) goto moonbr_io_exec_error;
-    if (dup2(sockerr[1], 2) == -1) goto moonbr_io_exec_error;
+    if (dup2(sockin[1], 0) == -1) goto moonbr_io_exec_error1;
+    if (dup2(sockout[1], 1) == -1) goto moonbr_io_exec_error1;
+    if (dup2(sockerr[1], 2) == -1) goto moonbr_io_exec_error1;
     closefrom(4);
-    if (fcntl(0, F_SETFD, 0) == -1) goto moonbr_io_exec_error;
-    if (fcntl(1, F_SETFD, 0) == -1) goto moonbr_io_exec_error;
-    if (fcntl(2, F_SETFD, 0) == -1) goto moonbr_io_exec_error;
+    if (fcntl(0, F_SETFD, 0) == -1) goto moonbr_io_exec_error1;
+    if (fcntl(1, F_SETFD, 0) == -1) goto moonbr_io_exec_error1;
+    if (fcntl(2, F_SETFD, 0) == -1) goto moonbr_io_exec_error1;
     if (execvp(argv[0], argv)) {
       errorcond = 2;
       strerror_r(errno, (char *)errmsgbuf, MOONBR_IO_MAXSTRERRORLEN);
       _exit(0);
     }
-    moonbr_io_exec_error:
+    moonbr_io_exec_error1:
     errorcond = 1;
     strerror_r(errno, (char *)errmsgbuf, MOONBR_IO_MAXSTRERRORLEN);
     _exit(0);
@@ -1312,29 +1330,49 @@
         luaL_error(L, "Error in waitpid call after unsuccessful exec: %s", errmsg);
       }
     }
+    child->pid = 0;
     lua_pushnil(L);
     if (errorcond == 2) lua_pushfstring(L, "Could not execute: %s", errmsgbuf);
     else lua_pushfstring(L, "Error in fork: %s", errmsgbuf);
     return 2;
   }
-  /* close sockets during garbage collection in case a Lua error is raised */
-  child->uninitialized_in = sockin[0];
-  child->uninitialized_out = sockout[0];
-  child->uninitialized_err = sockerr[0];
-  lua_newtable(L);
-  lua_setuservalue(L, -2);
-  luaL_getmetatable(L, MOONBR_IO_CHILD_MT_REGKEY);
-  lua_setmetatable(L, -2);
-  moonbr_io_pushhandle(L, sockin[0]);
+  lua_pushcfunction(L, moonbr_io_pushhandle_impl);
+  lua_pushlightuserdata(L, &sockin[0]);
+  if (lua_pcall(L, 1, 1, 0)) {
+    if (sockin[0] != -1) close(sockin[0]);
+    close(sockout[0]);
+    close(sockerr[0]);
+    goto moonbr_io_exec_error2;
+  }
   lua_setfield(L, -2, "stdin");
-  child->uninitialized_in = -1;
-  moonbr_io_pushhandle(L, sockout[0]);
+  lua_pushcfunction(L, moonbr_io_pushhandle_impl);
+  lua_pushlightuserdata(L, &sockout[0]);
+  if (lua_pcall(L, 1, 1, 0)) {
+    if (sockout[0] != -1) close(sockout[0]);
+    close(sockerr[0]);
+    goto moonbr_io_exec_error2;
+  }
   lua_setfield(L, -2, "stdout");
-  child->uninitialized_out = -1;
-  moonbr_io_pushhandle(L, sockerr[0]);
+  lua_pushcfunction(L, moonbr_io_pushhandle_impl);
+  lua_pushlightuserdata(L, &sockerr[0]);
+  if (lua_pcall(L, 1, 1, 0)) {
+    if (sockerr[0] != -1) close(sockerr[0]);
+    goto moonbr_io_exec_error2;
+  }
   lua_setfield(L, -2, "stderr");
-  child->uninitialized_err = -1;
   return 1;
+  moonbr_io_exec_error2:
+  {
+    int status;
+    while (waitpid(child->pid, &status, 0) == -1) {
+      if (errno != EINTR) {
+        moonbr_io_errmsg();
+        luaL_error(L, "Error in waitpid call after error creating socket handles: %s", errmsg);
+      }
+    }
+  }
+  child->pid = 0;
+  return lua_error(L);
 }
 
 static int moonbr_io_childindex(lua_State *L) {
@@ -1378,9 +1416,6 @@
       }
     }
   }
-  if (child->uninitialized_in != -1) close(child->uninitialized_in);
-  if (child->uninitialized_out != -1) close(child->uninitialized_out);
-  if (child->uninitialized_err != -1) close(child->uninitialized_err);
   return 0;
 }