From fb864b7cde40695c094363ee97c370ae2acc2e68 Mon Sep 17 00:00:00 2001 From: Frank Li Date: Fri, 11 Jan 2019 13:09:51 -0600 Subject: 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 Signed-off-by: Nathan Hjelm --- libusb/io.c | 2 ++ libusb/os/poll_posix.h | 3 +++ libusb/os/poll_windows.c | 63 ++++++++++++++++++++++++++++++++++++++++-------- libusb/os/poll_windows.h | 3 +++ libusb/version_nano.h | 2 +- 5 files changed, 62 insertions(+), 11 deletions(-) diff --git a/libusb/io.c b/libusb/io.c index 4c29046..1e9357c 100644 --- 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 --- 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 --- 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 --- 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 --- a/libusb/version_nano.h +++ b/libusb/version_nano.h @@ -1 +1 @@ -#define LIBUSB_NANO 11351 +#define LIBUSB_NANO 11352 -- cgit v1.2.1