Windows: Don't leak pipe fds
authorPete Batard <pbatard@gmail.com>
Tue, 8 Mar 2011 17:37:40 +0000 (17:37 +0000)
committerPeter Stuge <peter@stuge.se>
Mon, 13 Jun 2011 20:06:31 +0000 (22:06 +0200)
use _open() and _close() rather than _open_osfhandle() and CloseHandle()

* use of CloseHandle() prevented the pipe fds from being
  relinquished on libusb_exit()
* leaked fds could lead to the OS running out of new fds
  and LIBUSB_ERROR_NO_MEM being returned as a result
* using _open() avoids _open_osfhandle() redef for cygwin
* issue reported by Stephano Antonelli

libusb/os/poll_windows.c
libusb/os/poll_windows.h

index 556ac84..b17b253 100644 (file)
 
 #if defined(__CYGWIN__)
 // cygwin produces a warning unless these prototypes are defined
+extern int _open(char* name, int flags);
 extern int _close(int fd);
 extern int _snprintf(char *buffer, size_t count, const char *format, ...);
-extern int cygwin_attach_handle_to_fd(char *name, int fd, HANDLE handle, int bin, int access_mode);
-// _open_osfhandle() is not available on cygwin, but we can emulate
-// it for our needs with cygwin_attach_handle_to_fd()
-static inline int _open_osfhandle(intptr_t osfhandle, int flags)
-{
-       int access_mode;
-       switch (flags) {
-       case _O_RDONLY:
-               access_mode = GENERIC_READ;
-               break;
-       case _O_WRONLY:
-               access_mode = GENERIC_WRITE;
-               break;
-       case _O_RDWR:
-               access_mode = GENERIC_READ|GENERIC_WRITE;
-               break;
-       default:
-               usbi_err(NULL, "unsupported access mode");
-               return -1;
-       }
-       return cygwin_attach_handle_to_fd("/dev/null", -1, (HANDLE)osfhandle, -1, access_mode);
-}
+#define NUL_DEVICE "/dev/null"
+#else
+#define NUL_DEVICE "NUL"
 #endif
 
 #define CHECK_INIT_POLLING do {if(!is_polling_set) init_polling();} while(0)
@@ -288,7 +270,6 @@ void exit_polling(void)
 int usbi_pipe(int filedes[2])
 {
        int i;
-       HANDLE handle;
        OVERLAPPED* overlapped;
 
        CHECK_INIT_POLLING;
@@ -302,12 +283,11 @@ int usbi_pipe(int filedes[2])
        overlapped->InternalHigh = 0;
 
        // Read end of the "pipe"
-       handle = CreateFileA("NUL", 0, 0, NULL, OPEN_EXISTING, 0, NULL);
-       if (handle == INVALID_HANDLE_VALUE) {
-               usbi_err(NULL, "could not create pipe: errcode %d", (int)GetLastError());
+       filedes[0] = _open(NUL_DEVICE, _O_WRONLY);
+       if (filedes[0] < 0) {
+               usbi_err(NULL, "could not create pipe: errno %d", errno);
                goto out1;
        }
-       filedes[0] = _open_osfhandle((intptr_t)handle, _O_RDONLY);
        // We can use the same handle for both ends
        filedes[1] = filedes[0];
        poll_dbg("pipe filedes = %d", filedes[0]);
@@ -328,7 +308,7 @@ int usbi_pipe(int filedes[2])
                        }
 
                        poll_fd[i].fd = filedes[0];
-                       poll_fd[i].handle = handle;
+                       poll_fd[i].handle = DUMMY_HANDLE;
                        poll_fd[i].overlapped = overlapped;
                        // There's no polling on the write end, so we just use READ for our needs
                        poll_fd[i].rw = RW_READ;
@@ -340,7 +320,7 @@ int usbi_pipe(int filedes[2])
 
        CloseHandle(overlapped->hEvent);
 out2:
-       CloseHandle(handle);
+       _close(filedes[0]);
 out1:
        free(overlapped);
        return -1;
@@ -383,9 +363,9 @@ struct winfd usbi_create_fd(HANDLE handle, int access_mode)
                wfd.rw = RW_WRITE;
        }
 
-       // Ensure that we get a non system conflicting unique fd
-       fd = _open_osfhandle((intptr_t)CreateFileA("NUL", 0, 0,
-               NULL, OPEN_EXISTING, 0, NULL), _O_RDWR);
+       // Ensure that we get a non system conflicting unique fd, using
+       // the same fd attribution system as the pipe ends
+       fd = _open(NUL_DEVICE, _O_WRONLY);
        if (fd < 0) {
                return INVALID_WINFD;
        }
@@ -770,10 +750,9 @@ int usbi_close(int fd)
                        CloseHandle(poll_fd[_index].overlapped->hEvent);
                        free(poll_fd[_index].overlapped);
                }
-               if (CloseHandle(poll_fd[_index].handle) == 0) {
+               r = _close(poll_fd[_index].fd);
+               if (r != 0) {
                        errno = EIO;
-               } else {
-                       r = 0;
                }
                poll_fd[_index] = INVALID_WINFD;
                LeaveCriticalSection(&_poll_fd[_index].mutex);
index a4d599d..fee89f5 100644 (file)
@@ -38,6 +38,8 @@
 #define STATUS_COMPLETED_SYNCHRONOUSLY STATUS_REPARSE
 #define HasOverlappedIoCompletedSync(lpOverlapped)     (((DWORD)(lpOverlapped)->Internal) == STATUS_COMPLETED_SYNCHRONOUSLY)
 
+#define DUMMY_HANDLE ((HANDLE)(LONG_PTR)-2)
+
 enum windows_version {
        WINDOWS_UNSUPPORTED,
        WINDOWS_XP,
@@ -70,10 +72,10 @@ enum rw_type {
 
 // fd struct that can be used for polling on Windows
 struct winfd {
-       int fd;                                     // what's exposed to libusb core
-       HANDLE handle;                  // what we need to attach overlapped to the I/O op, so we can poll it
-       OVERLAPPED* overlapped;         // what will report our I/O status
-       enum rw_type rw;                // I/O transfer direction: read *XOR* write (NOT BOTH)
+       int fd;                                                 // what's exposed to libusb core
+       HANDLE handle;                                  // what we need to attach overlapped to the I/O op, so we can poll it
+       OVERLAPPED* overlapped;                 // what will report our I/O status
+       enum rw_type rw;                                // I/O transfer direction: read *XOR* write (NOT BOTH)
 };
 extern const struct winfd INVALID_WINFD;