fix windows crash when multi-thread do sync transfer

fun()
{
    libusb_open()
    ... sync transfer
    libusb_close()
}

two thread call fun infininately.

to speed up crash, enable application verifier

below 20 cycle, assert(fd!=NULL) happen at check_pollfds
below 100 cycle, crash at pollable_fd->overlappend in
winusb_get_overlapped result

with this fix, success fun over 1000 cycles

in handle_events

usbi_mutex_lock()
fds = ctx->pollfds
nfds = ctx->pollfds_cnt;
usbi_mutex_unclock()

usbi_poll()
callback.

usbi poll is not in mutex. pollfds may be change by usbi_add_pollfd
and usbi_remove_pollfd.

Although usbi_add_pollfd and usbi_remove_pollfd hold mutex, but
usbi_poll and callback is not in protext of mutex.

windows use fd as index of fb_table. fb_table may changed by
usbi_remove_pollfd.  the map between fd and internal file_descriptor may
be wrong.

this patch added ref count for file_descriptor, only free file_desciptor
and remove it from fb_table when ref count is 0.

ref count will be increase when fds copy with mutex lock.
so fd always index validate file_descriptor.

ref count will be descress before return from handle_events.
the file_descriptor can be free safely at this time.

Closes #521

Signed-off-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Nathan Hjelm <hjelmn@me.com>

diff --git a/libusb/io.c b/libusb/io.c
index 4c29046..1e9357c 100644 (file)
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -2149,6 +2149,7 @@ static int handle_events(struct libusb_context *ctx, struct timeval *tv)
        }
        fds = ctx->pollfds;
        nfds = ctx->pollfds_cnt;
+       usbi_inc_fds_ref(fds, nfds);
        usbi_mutex_unlock(&ctx->event_data_lock);
 
        timeout_ms = (int)(tv->tv_sec * 1000) + (tv->tv_usec / 1000);
@@ -2281,6 +2282,7 @@ static int handle_events(struct libusb_context *ctx, struct timeval *tv)
 
 done:
        usbi_end_event_handling(ctx);
+       usbi_dec_fds_ref(fds, nfds);
        return r;
 }
 

diff --git a/libusb/os/poll_posix.h b/libusb/os/poll_posix.h
index 5b4b2c9..bc0239c 100644 (file)
--- a/libusb/os/poll_posix.h
+++ b/libusb/os/poll_posix.h
@@ -8,4 +8,7 @@
 
 int usbi_pipe(int pipefd[2]);
 
+#define usbi_inc_fds_ref(x, y)
+#define usbi_dec_fds_ref(x, y)
+
 #endif /* LIBUSB_POLL_POSIX_H */

diff --git a/libusb/os/poll_windows.c b/libusb/os/poll_windows.c
index 4d28333..208953b 100644 (file)
--- a/libusb/os/poll_windows.c
+++ b/libusb/os/poll_windows.c
@@ -50,6 +50,7 @@ const struct winfd INVALID_WINFD = { -1, NULL };
 struct file_descriptor {
        enum fd_type { FD_TYPE_PIPE, FD_TYPE_TRANSFER } type;
        OVERLAPPED overlapped;
+       int refcount;
 };
 
 static usbi_mutex_static_t fd_table_lock = USBI_MUTEX_INITIALIZER;
@@ -66,6 +67,7 @@ static struct file_descriptor *create_fd(enum fd_type type)
                return NULL;
        }
        fd->type = type;
+       fd->refcount = 1;
        return fd;
 }
 
@@ -117,6 +119,43 @@ struct winfd usbi_create_fd(void)
        return wfd;
 }
 
+int usbi_inc_fds_ref(struct pollfd *fds, unsigned int nfds)
+{
+       int n;
+       usbi_mutex_static_lock(&fd_table_lock);
+       for (n = 0; n < nfds; ++n) {
+               fd_table[fds[n].fd]->refcount++;
+       }
+       usbi_mutex_static_unlock(&fd_table_lock);
+}
+
+int usbi_dec_fds_ref(struct pollfd *fds, unsigned int nfds)
+{
+       int n;
+       struct file_descriptor *fd;
+
+       usbi_mutex_static_lock(&fd_table_lock);
+       for (n = 0; n < nfds; ++n) {
+               fd = fd_table[fds[n].fd];
+               fd->refcount--;
+               if (fd->refcount == 0)
+               {
+                       if (fd->type == FD_TYPE_PIPE) {
+                               // InternalHigh is our reference count
+                               fd->overlapped.InternalHigh--;
+                               if (fd->overlapped.InternalHigh == 0)
+                                       free_fd(fd);
+                       }
+                       else {
+                               free_fd(fd);
+                       }
+                       fd_table[fds[n].fd] = NULL;
+               }
+       }
+       usbi_mutex_static_unlock(&fd_table_lock);
+}
+
+
 static int check_pollfds(struct pollfd *fds, unsigned int nfds,
        HANDLE *wait_handles, DWORD *nb_wait_handles)
 {
@@ -209,21 +248,25 @@ int usbi_close(int _fd)
 
        usbi_mutex_static_lock(&fd_table_lock);
        fd = fd_table[_fd];
-       fd_table[_fd] = NULL;
+       fd->refcount--;
+       if(fd->refcount==0)
+       {       fd_table[_fd] = NULL;
+
+               if (fd->type == FD_TYPE_PIPE) {
+                       // InternalHigh is our reference count
+                       fd->overlapped.InternalHigh--;
+                       if (fd->overlapped.InternalHigh == 0)
+                               free_fd(fd);
+               }
+               else {
+                       free_fd(fd);
+               }
+       }
        usbi_mutex_static_unlock(&fd_table_lock);
 
        if (fd == NULL)
                goto err_badfd;
 
-       if (fd->type == FD_TYPE_PIPE) {
-               // InternalHigh is our reference count
-               fd->overlapped.InternalHigh--;
-               if (fd->overlapped.InternalHigh == 0)
-                       free_fd(fd);
-       } else {
-               free_fd(fd);
-       }
-
        return 0;
 
 err_badfd:

diff --git a/libusb/os/poll_windows.h b/libusb/os/poll_windows.h
index bd22c7f..980870d 100644 (file)
--- a/libusb/os/poll_windows.h
+++ b/libusb/os/poll_windows.h
@@ -71,6 +71,9 @@ ssize_t usbi_write(int fd, const void *buf, size_t count);
 ssize_t usbi_read(int fd, void *buf, size_t count);
 int usbi_close(int fd);
 
+int usbi_inc_fds_ref(struct pollfd *fds, unsigned int nfds);
+int usbi_dec_fds_ref(struct pollfd *fds, unsigned int nfds);
+
 /*
  * Timeval operations
  */

diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index af03632..ada3860 100644 (file)
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11351
+#define LIBUSB_NANO 11352