Re: [PATCH v5 0/1] MR1503: wineusb.sys: Move event handling to a single thread.

[Zebediah Figura (＠zfigura)]

> > Is it guaranteed that libusb_handle_events() will always return immediately after running an event callback?
>
> Yes. According to the libusb_handle_events_timeout_completed() documentation, which is the main function used internally by all other libusb_handle_events variant functions - *"If a non-zero timeval is passed and no events are currently pending, this function will block waiting for events to handle up until the specified timeout. If an event arrives or a signal is raised, this function will return early."*
>
> libusb_handle_events() basically calls libusb_handle_events_timeout_completed() with a 60s timeval, so it'll return either when an event occurs (hotplug, transfer, err, exit, etc.) or the timeout expires. I observed all the libusb code related to libusb_handle_events(), and found it does exactly that.

Thanks, I did briefly look at the documentation but missed that.
> > I'm also disturbed by the fact that this fixes anything. Partly because I don't see what's special about that thread, and partly because I don't know why we're getting SIGQUIT?
>
> This core dump occurs when the last application quits, so a SIGQUIT is sent to all remaining services to quit. This particular winedevice.exe (which shares both winebus.sys and wineusb.sys) receives the signal, and when each thread's SIGQUIT handler gets called, the usb event handling thread segfaults somewhere in pthread_cond_wait() while being canceled by pthread_exit().
>
> Normally this should work, but something happens when the library is implemented in Wine's Windows/Unix style. According to the bug report, when the execution flow is in the Unix library, it resets the stack to the exit_frame and calls the pthread_exit(), which fails to unwind properly. It's well described in this and the following post of the report: https://bugs.winehq.org/show_bug.cgi?id=52213#c11
>
> So, to workaround this I just avoided using pthread_cond_wait() and moved to single-threaded event handling, since the libusb_handle_events() suits this approach quite well. Followed similar logic to the winebus.sys event thread implementation.

Wouldn't we normally be in libusb_handle_events() at this point? I guess there's just differences between different call stacks?

I don't think we should be terminating threads at all. It appears that comes from 7f749d58d, but we should probably just figure out what's keeping those threads alive and give them a chance to shut down naturally, adjusting timeouts if necessary.

That's all orthogonal to this change, though, or at least, I think it makes sense regardless of bug 52213, since it simplifies the code.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1503#note_17734