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!");
 
 }
-- 
1.7.2.3


------------------------------------------------------------------------------
EditLive Enterprise is the world's most technically advanced content
authoring tool. Experience the power of Track Changes, Inline Image
Editing and ensure content is compliant with Accessibility Checking.
http://p.sf.net/sfu/ephox-dev2dev
_______________________________________________
Barry-devel mailing list
Barry-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/barry-devel

Reply via email to