Hello,
Sorry for the looooong time it took me to test your patch.
It worked well for me both on my arm target and on my desktop.
Thank you !
On 06/20/11 22:59, Chris Frey wrote:
> On Tue, Jun 14, 2011 at 03:13:21PM +0200, Nicolas CARRIER wrote:
>> I from time to time, had pppob not terminating properly. As I couldn't
>> figure out why, I tried to suppress possible causes.
>> What is sure is that in some bad timing circumstances, the signal could
>> be handled right before the select statement hence resulting in a
>> useless 30 seconds timeout, which this kind of patch should avoid.
> I did a bit of testing here, and it seems to consistently hang in the
> pthread_join() in the IpModem::Close() function.
>
> Not sure exactly why, but it seems that having two threads calling
> libusb's bulk read might be problematic. Could you give this patch
> a try, and let me know if it fixes the problem?
>
> Thanks,
> - Chris
>
>
> Subject: [PATCH] lib: fixed hang in IPModem Close
>
> If the data read thread is still going, then there is no need to do
> an explicit BulkRead() in the Close() routine. Only do this if the
> thread is not running.
>
> When two libusb reads are done at the same time on the same endpoint
> from different threads, it seems like it causes a hang in the pthread_join().
> Not sure why.
> ---
> src/m_ipmodem.cc | 21 +++++++++++++--------
> 1 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/src/m_ipmodem.cc b/src/m_ipmodem.cc
> index b7ea0ec..6b286c3 100644
> --- a/src/m_ipmodem.cc
> +++ b/src/m_ipmodem.cc
> @@ -414,19 +414,24 @@ void IpModem::Close()
> memcpy(&end[24], special_flag, sizeof(special_flag));
> m_dev.BulkWrite(write_ep, end, sizeof(end));
> m_dev.BulkWrite(write_ep, stop, sizeof(stop));
> - try {
> - m_dev.BulkRead(read_ep, data, 5000);
> - ddout("IPModem: Close read packet:\n"<< data);
> - }
> - catch( Usb::Timeout&to ) {
> - // do nothing on timeouts
> - ddout("IPModem: Close Read Timeout");
> - }
> +
> // stop the read thread
> if( m_continue_reading ) {
> m_continue_reading = false;
> pthread_join(m_modem_read_thread, NULL);
> }
> + else {
> + // otherwise, drain the last read
> + try {
> + m_dev.BulkRead(read_ep, data, 5000);
> + ddout("IPModem: Close read packet:\n"<< data);
> + }
> + catch( Usb::Timeout&to ) {
> + // do nothing on timeouts
> + ddout("IPModem: Close Read Timeout");
> + }
> + }
> +
> ddout("IPmodem: Closed!");
>
> }
--
Nicolas CARRIER - 6081 - 2ème centre
------------------------------------------------------------------------------
All of the data generated in your IT infrastructure is seriously valuable.
Why? It contains a definitive record of application performance, security
threats, fraudulent activity, and more. Splunk takes this data and makes
sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-d2d-c2
_______________________________________________
Barry-devel mailing list
Barry-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/barry-devel
