Re: [Libusb-devel] event handling and canceled thread issue (again)

[Ludovic R. <lud...@gm...>]

2008/12/2 Daniel Drake <da...@re...>:
> Ludovic Rousseau wrote:
>> I try to use libusb-0.9.x + libusb-compat with pilot-link [1]
>> (pilot-link is a library used to talk to a Palm PDA).
>>
>> I discovered that I have the exact same problem I described in [2].
>> The problem is that pilot-link is creating a thread to manage USB
>> communication (function RD_main line 315 of [3]). When the
>> communication is finished this thread is thread_cancel()ed and
>> usb_close() is called from the main thread.
>
> My response has not changed from last time either.
> http://article.gmane.org/gmane.comp.lib.libusb.devel.general/4677
>
> Don't use thread_cancel(). It's bad programming practice. For example you
> might call thread_cancel() while the thread you are killing has malloc()ed
> some memory, causing that memory to never be freed. Implement a way of
> signalling your threads to shut down cleanly.
>
> Regardless of my complaints against your pratcitces, I will happily consider
> a *clean* way of implementing this but your proposed solution of using
> pthread_kill() to check thread existence is not clean. This is because when
> threads die holding a mutex, the mutex is still held, and therefore your
> patch causes a thread (that doesn't own the mutex) to unlock it, which the
> pthread manual explicitly says is illegal.
>
> For now I don't have any other solution or ideas, sorry. But feel free to
> suggest other approaches to this.

I think I found an elegant way to solve this problem.

The patch "Pause event handling while opening and closing devices" [1]
applied to solve another problem I reported [2] now causes any pending
libusb_read() (any event handler in fact) to return when
libusb_close() is called in another thread.

The reading thread is now "interrupted" and pthread_cancel() is not
needed anymore to stop it.

The patch for pilot-link is really simple. Just remove the call to
pthread_cancel() and that's it. I will try the same approach for my
CCID driver. I don't see why it could not work.

Maybe a more general libusb_interrupt_event_handling() is missing in
the API. But I don't need this feature for now.

Regards,

[1] http://projects.reactivated.net/cgi-bin/gitweb.cgi?p=libusb.git;a=commitdiff;h=c32aa662769b676ff3247778664fccc71fc427ec
[2] http://thread.gmane.org/gmane.comp.lib.libusb.devel.general/5174

-- 
 Dr. Ludovic Rousseau