simplify libT locks now that it's (more-or-less) single-threaded. fix deadlocks. make tr_locks nestable.

libtransmission/internal.h

@@ -139,7 +139,6 @@ struct tr_torrent
     run_status_t               runStatus;
     run_status_t               runStatusToSave;
     cp_status_t                cpStatus;
-    struct tr_lock           * lock;
 
     struct tr_tracker        * tracker;
     struct tr_publisher_tag  * trackerSubscription;
@@ -188,6 +187,8 @@ struct tr_handle
     struct tr_peerMgr        * peerMgr;
     struct tr_shared         * shared;
 
+    struct tr_lock           * lock;
+
     tr_handle_status           stats[2];
     int                        statCur;
 
@@ -197,4 +198,9 @@ struct tr_handle
     uint8_t                    azId[TR_AZ_ID_LEN];
 };
 
+void tr_globalLock       ( struct tr_handle * );
+void tr_globalUnlock     ( struct tr_handle * );
+int  tr_globalIsLocked   ( const struct tr_handle * );
+
+
 #endif

libtransmission/makemeta.c

@@ -415,10 +415,10 @@ static tr_lock* getQueueLock( tr_handle * h )
 {
     static tr_lock * lock = NULL;
 
-    tr_sharedLock( h->shared );
+    tr_globalLock( h );
     if( !lock )
          lock = tr_lockNew( );
-    tr_sharedUnlock( h->shared );
+    tr_globalUnlock( h );
 
     return lock;
 }

libtransmission/peer-mgr.c

@@ -29,11 +29,10 @@
 #include "platform.h"
 #include "ptrarray.h"
 #include "ratecontrol.h"
+#include "shared.h"
 #include "trevent.h"
 #include "utils.h"
 
-#include "pthread.h"
-
 enum
 {
     /* how frequently to change which peers are choked */
@@ -106,8 +105,6 @@ struct tr_peerMgr
     tr_ptrArray * torrents; /* Torrent */
     int connectionCount;
     tr_ptrArray * handshakes; /* in-process */
-    tr_lock * lock;
-    pthread_t lockThread;
 };
 
 /**
@@ -117,16 +114,12 @@ struct tr_peerMgr
 static void
 managerLock( struct tr_peerMgr * manager )
 {
-    assert( manager->lockThread != pthread_self() );
-    tr_lockLock( manager->lock );
-    manager->lockThread = pthread_self();
+    tr_globalLock( manager->handle );
 }
 static void
 managerUnlock( struct tr_peerMgr * manager )
 {
-    assert( manager->lockThread == pthread_self() );
-    manager->lockThread = 0;
-    tr_lockUnlock( manager->lock );
+    tr_globalUnlock( manager->handle );
 }
 static void
 torrentLock( Torrent * torrent )
@@ -141,10 +134,8 @@ torrentUnlock( Torrent * torrent )
 static int
 torrentIsLocked( const Torrent * t )
 {
-    return ( t != NULL )
-        && ( t->manager != NULL )
-        && ( t->manager->lockThread != 0 )
-        && ( t->manager->lockThread == pthread_self( ) );
+    return ( t != NULL ) 
+        && ( tr_globalIsLocked( t->manager->handle ) );
 }
 
 /**
@@ -435,7 +426,6 @@ tr_peerMgrNew( tr_handle * handle )
     m->handle = handle;
     m->torrents = tr_ptrArrayNew( );
     m->handshakes = tr_ptrArrayNew( );
-    m->lock = tr_lockNew( );
     return m;
 }
 
@@ -448,7 +438,6 @@ tr_peerMgrFree( tr_peerMgr * manager )
     tr_ptrArrayFree( manager->torrents, (PtrArrayForeachFunc)freeTorrent );
 
     managerUnlock( manager );
-    tr_lockFree( manager->lock );
     tr_free( manager );
 }
 
@@ -482,7 +471,7 @@ struct tr_refill_piece
     uint32_t piece;
     uint32_t peerCount;
     uint32_t fastAllowed;
-    uint32_t random;
+    uint8_t random;
 };
 
 static int
@@ -504,7 +493,7 @@ compareRefillPiece (const void * aIn, const void * bIn)
         return a->fastAllowed < b->fastAllowed ? -1 : 1;
 
     /* otherwise go with our random seed */
-    return tr_compareUint32( a->random, b->random );
+    return tr_compareUint8( a->random, b->random );
 }
 
 static int
@@ -556,7 +545,7 @@ getPreferredPieces( Torrent     * t,
             setme->priority = inf->pieces[piece].priority;
             setme->peerCount = 0;
             setme->fastAllowed = 0;
-            setme->random = tr_rand( UINT32_MAX );
+            setme->random = tr_rand( UINT8_MAX );
             /* FIXME */
 //            setme->fastAllowed = tr_bitfieldHas( t->tor->allowedList, i);
 
@@ -819,10 +808,8 @@ msgsCallbackFunc( void * vpeer, void * vevent, void * vt )
     tr_peer * peer = vpeer;
     Torrent * t = (Torrent *) vt;
     const tr_peermsgs_event * e = (const tr_peermsgs_event *) vevent;
-    const int needLock = !torrentIsLocked( t );
 
-    if( needLock )
-        torrentLock( t );
+    torrentLock( t );
 
     switch( e->eventType )
     {
@@ -861,8 +848,7 @@ msgsCallbackFunc( void * vpeer, void * vevent, void * vt )
             assert(0);
     }
 
-    if( needLock )
-        torrentUnlock( t );
+    torrentUnlock( t );
 }
 
 static void
@@ -964,7 +950,6 @@ initiateHandshake( tr_peerMgr * manager, tr_peerIo * io )
 {
     tr_handshake * handshake;
 
-    assert( manager->lockThread!=0 );
     assert( io != NULL );
 
     handshake = tr_handshakeNew( io,
@@ -1091,14 +1076,12 @@ tr_peerMgrGetPeers( tr_peerMgr      * manager,
                     tr_pex         ** setme_pex )
 {
     const Torrent * t = getExistingTorrent( (tr_peerMgr*)manager, torrentHash );
-    const int isLocked = torrentIsLocked( t );
     int i, peerCount;
     const tr_peer ** peers;
     tr_pex * pex;
     tr_pex * walk;
 
-    if( !isLocked )
-        torrentLock( (Torrent*)t );
+    torrentLock( (Torrent*)t );
 
     peers = (const tr_peer **) tr_ptrArrayPeek( t->peers, &peerCount );
     pex = walk = tr_new( tr_pex, peerCount );
@@ -1120,8 +1103,7 @@ tr_peerMgrGetPeers( tr_peerMgr      * manager,
     qsort( pex, peerCount, sizeof(tr_pex), tr_pexCompare );
     *setme_pex = pex;
 
-    if( !isLocked )
-        torrentUnlock( (Torrent*)t );
+    torrentUnlock( (Torrent*)t );
 
     return peerCount;
 }

libtransmission/platform.c

@@ -56,6 +56,38 @@
 ****  THREADS
 ***/
 
+#ifdef __BEOS__
+typedef thread_id tr_thread_id;
+#elif defined(WIN32)
+typedef DWORD tr_thread_id;
+#else
+typedef pthread_t tr_thread_id;
+#endif
+
+static tr_thread_id
+tr_getCurrentThread( void )
+{
+#ifdef __BEOS__
+    return find_thread( NULL );
+#elif defined(WIN32)
+    return GetCurrentThreadId();
+#else
+    return pthread_self( );
+#endif
+}
+
+static int
+tr_areThreadsEqual( tr_thread_id a, tr_thread_id b )
+{
+#ifdef __BEOS__
+    return a == b;
+#elif defined(WIN32)
+    return a == b;
+#else
+    return pthread_equal( a, b );
+#endif
+}
+
 struct tr_thread
 {
     void          (* func ) ( void * );
@@ -73,6 +105,12 @@ struct tr_thread
 
 };
 
+int
+tr_amInThread ( const tr_thread * t )
+{
+    return tr_areThreadsEqual( tr_getCurrentThread(), t->thread );
+}
+
 #ifdef WIN32
 #define ThreadFuncReturnType unsigned WINAPI
 #else
@@ -122,18 +160,6 @@ tr_threadNew( void (*func)(void *),
 
     return t;
 }
-
-int
-tr_amInThread ( const tr_thread * t )
-{
-#ifdef __BEOS__
-    return find_thread(NULL) == t->thread;
-#elif defined(WIN32)
-    return GetCurrentThreadId() == t->thread_id;
-#else
-    return pthread_equal( t->thread, pthread_self( ) );
-#endif
-}
     
 void
 tr_threadJoin( tr_thread * t )
@@ -163,12 +189,16 @@ tr_threadJoin( tr_thread * t )
 
 struct tr_lock
 {
+    uint32_t depth;
 #ifdef __BEOS__
     sem_id lock;
+    thread_id lockThread;
 #elif defined(WIN32)
     CRITICAL_SECTION lock;
+    DWORD lockThread;
 #else
     pthread_mutex_t lock;
+    pthread_t lockThread;
 #endif
 };
 
@@ -216,25 +246,48 @@ tr_lockTryLock( tr_lock * l ) /* success on zero! */
 void
 tr_lockLock( tr_lock * l )
 {
+    tr_thread_id currentThread = tr_getCurrentThread( );
+    if( l->lockThread == currentThread )
+    {
+        ++l->depth;
+    }
+    else
+    {
 #ifdef __BEOS__
-    acquire_sem( l->lock );
+        acquire_sem( l->lock );
 #elif defined(WIN32)
-    EnterCriticalSection( &l->lock );
+        EnterCriticalSection( &l->lock );
 #else
-    pthread_mutex_lock( &l->lock );
+        pthread_mutex_lock( &l->lock );
 #endif
+        l->lockThread = tr_getCurrentThread( );
+        l->depth = 1;
+    }
+}
+
+int
+tr_lockHave( const tr_lock * l )
+{
+    return ( l->depth > 0 )
+        && ( l->lockThread == tr_getCurrentThread() );
 }
 
 void
 tr_lockUnlock( tr_lock * l )
 {
+    assert( tr_lockHave( l ) );
+
+    if( !--l->depth )
+    {
+        l->lockThread = 0;
 #ifdef __BEOS__
-    release_sem( l->lock );
+        release_sem( l->lock );
 #elif defined(WIN32)
-    LeaveCriticalSection( &l->lock );
+        LeaveCriticalSection( &l->lock );
 #else
-    pthread_mutex_unlock( &l->lock );
+        pthread_mutex_unlock( &l->lock );
 #endif
+    }
 }
 
 /***

libtransmission/platform.h

@@ -41,6 +41,7 @@ void         tr_lockFree       ( tr_lock * );
 int          tr_lockTryLock    ( tr_lock * );
 void         tr_lockLock       ( tr_lock * );
 void         tr_lockUnlock     ( tr_lock * );
+int          tr_lockHave       ( const tr_lock * );
 
 tr_cond *    tr_condNew       ( void );
 void         tr_condFree      ( tr_cond * );

libtransmission/shared.c

@@ -44,7 +44,6 @@
 struct tr_shared
 {
     tr_handle    * h;
-    tr_lock      * lock;
     tr_timer     * pulseTimer;
 
     /* Incoming connections */
@@ -75,7 +74,6 @@ tr_shared * tr_sharedInit( tr_handle * h )
     tr_shared * s = calloc( 1, sizeof( tr_shared ) );
 
     s->h          = h;
-    s->lock       = tr_lockNew( );
     s->publicPort = -1;
     s->bindPort   = -1;
     s->bindSocket = -1;
@@ -96,25 +94,11 @@ void tr_sharedClose( tr_shared * s )
     tr_timerFree( &s->pulseTimer );
 
     tr_netClose( s->bindSocket );
-    tr_lockFree( s->lock );
     tr_natpmpClose( s->natpmp );
     tr_upnpClose( s->upnp );
     free( s );
 }
 
-/**
-***
-**/
-
-void tr_sharedLock( tr_shared * s )
-{
-    tr_lockLock( s->lock );
-}
-void tr_sharedUnlock( tr_shared * s )
-{
-    tr_lockUnlock( s->lock );
-}
-
 /***********************************************************************
  * tr_sharedSetPort
  ***********************************************************************
@@ -128,11 +112,11 @@ void tr_sharedSetPort( tr_shared * s, int port )
     return;
 #endif
 
-    tr_sharedLock( s );
+    tr_globalLock( s->h );
 
     if( port == s->bindPort )
     {
-        tr_sharedUnlock( s );
+        tr_globalUnlock( s->h );
         return;
     }
     s->bindPort = port;
@@ -165,7 +149,7 @@ void tr_sharedSetPort( tr_shared * s, int port )
         tr_upnpForwardPort( s->upnp, port );
     }
 
-    tr_sharedUnlock( s );
+    tr_globalUnlock( s->h );
 }
 
 int
@@ -237,7 +221,7 @@ SharedLoop( void * vs )
     int newPort;
     tr_shared * s = vs;
 
-    tr_sharedLock( s );
+    tr_globalLock( s->h );
 
     /* NAT-PMP and UPnP pulses */
     newPort = -1;
@@ -249,7 +233,7 @@ SharedLoop( void * vs )
     /* Handle incoming connections */
     AcceptPeers( s );
 
-    tr_sharedUnlock( s );
+    tr_globalUnlock( s->h );
 
     return TRUE;
 }

libtransmission/shared.h

@@ -38,15 +38,6 @@ typedef struct tr_shared tr_shared;
 tr_shared * tr_sharedInit           ( tr_handle * );
 void        tr_sharedClose          ( tr_shared * );
 
-/***********************************************************************
- * tr_sharedLock, tr_sharedUnlock
- ***********************************************************************
- * Gets / releases exclusive access to ressources used by the shared
- * thread
- **********************************************************************/
-void          tr_sharedLock           ( tr_shared * );
-void          tr_sharedUnlock         ( tr_shared * );
-
 /***********************************************************************
  * tr_sharedSetPort
  ***********************************************************************

libtransmission/torrent.c

@@ -90,13 +90,13 @@ tr_torrentFindFromObfuscatedHash( tr_handle      * handle,
 void
 tr_torrentLock( const tr_torrent * tor )
 {
-    tr_lockLock ( (tr_lock*)tor->lock );
+    tr_globalLock( tor->handle );
 }
 
 void
 tr_torrentUnlock( const tr_torrent * tor )
 {
-    tr_lockUnlock ( (tr_lock*)tor->lock );
+    tr_globalUnlock( tor->handle );
 }
 
 /***
@@ -299,7 +299,7 @@ torrentRealInit( tr_handle  * h,
     
     tor->info.flags |= flags;
 
-    tr_sharedLock( h->shared );
+    tr_globalLock( h );
 
     tor->destination = tr_strdup( destination );
 
@@ -355,8 +355,6 @@ torrentRealInit( tr_handle  * h,
 
     tr_torrentInitFilePieces( tor );
 
-    tor->lock = tr_lockNew( );
-
     tor->upload         = tr_rcInit();
     tor->download       = tr_rcInit();
     tor->swarmspeed     = tr_rcInit();
@@ -364,8 +362,6 @@ torrentRealInit( tr_handle  * h,
     tr_sha1( tor->obfuscatedHash, "req2", 4,
                                   info->hash, SHA_DIGEST_LENGTH,
                                   NULL );
- 
-    tr_sharedUnlock( h->shared );
 
     tr_peerMgrAddTorrent( h->peerMgr, tor );
 
@@ -407,11 +403,11 @@ torrentRealInit( tr_handle  * h,
     tor->tracker = tr_trackerNew( tor );
     tor->trackerSubscription = tr_trackerSubscribe( tor->tracker, onTrackerResponse, tor );
 
-    tr_sharedLock( h->shared );
     tor->next = h->torrentList;
     h->torrentList = tor;
     h->torrentCount++;
-    tr_sharedUnlock( h->shared );
+
+    tr_globalUnlock( h );
 
     tr_ioRecheckAdd( tor, recheckDoneCB, tor->runStatus );
 }
@@ -999,11 +995,10 @@ tr_torrentFree( tr_torrent * tor )
     assert( tor != NULL );
     assert( tor->runStatus == TR_RUN_STOPPED );
 
-    tr_sharedLock( h->shared );
+    tr_globalLock( h );
 
     tr_peerMgrRemoveTorrent( h->peerMgr, tor->info.hash );
 
-    tr_lockFree( tor->lock );
     tr_cpClose( tor->completion );
 
     tr_rcClose( tor->upload );
@@ -1034,7 +1029,7 @@ tr_torrentFree( tr_torrent * tor )
     tr_metainfoFree( inf );
     tr_free( tor );
 
-    tr_sharedUnlock( h->shared );
+    tr_globalUnlock( h );
 }
 
 static int

libtransmission/transmission.c

@@ -121,6 +121,8 @@ tr_handle * tr_init( const char * tag )
     if( !h )
         return NULL;
 
+    h->lock = tr_lockNew( );
+
     h->encryptionMode = TR_ENCRYPTION_PREFERRED;
 
     tr_netInit(); /* must go before tr_eventInit */
@@ -153,6 +155,28 @@ tr_handle * tr_init( const char * tag )
     return h;
 }
 
+/***
+****
+***/
+
+void
+tr_globalLock( struct tr_handle * handle )
+{
+    tr_lockLock( handle->lock );
+}
+
+void
+tr_globalUnlock( struct tr_handle * handle )
+{
+    tr_lockUnlock( handle->lock );
+}
+
+int
+tr_globalIsLocked( const struct tr_handle * handle )
+{
+    return tr_lockHave( handle->lock );
+}
+
 /***********************************************************************
  * tr_setBindPort
  ***********************************************************************
@@ -173,9 +197,9 @@ tr_getPublicPort( const tr_handle * h )
 
 void tr_natTraversalEnable( tr_handle * h, int enable )
 {
-    tr_sharedLock( h->shared );
+    tr_globalLock( h );
     tr_sharedTraversalEnable( h->shared, enable );
-    tr_sharedUnlock( h->shared );
+    tr_globalUnlock( h );
 }
 
 tr_handle_status * tr_handleStatus( tr_handle * h )
@@ -185,12 +209,12 @@ tr_handle_status * tr_handleStatus( tr_handle * h )
     h->statCur = ( h->statCur + 1 ) % 2;
     s = &h->stats[h->statCur];
 
-    tr_sharedLock( h->shared );
+    tr_globalLock( h );
 
     s->natTraversalStatus = tr_sharedTraversalStatus( h->shared );
     s->publicPort = tr_sharedGetPublicPort( h->shared );
 
-    tr_sharedUnlock( h->shared );
+    tr_globalUnlock( h );
 
     return s;
 }
@@ -241,7 +265,7 @@ tr_torrentRates( tr_handle * h, float * dl, float * ul )
 
     *dl = 0.0;
     *ul = 0.0;
-    tr_sharedLock( h->shared );
+    tr_globalLock( h );
     for( tor = h->torrentList; tor; tor = tor->next )
     {
         tr_torrentLock( tor );
@@ -250,7 +274,7 @@ tr_torrentRates( tr_handle * h, float * dl, float * ul )
         *ul += tr_rcRate( tor->upload );
         tr_torrentUnlock( tor );
     }
-    tr_sharedUnlock( h->shared );
+    tr_globalUnlock( h );
 }
 
 int
@@ -300,6 +324,7 @@ tr_close( tr_handle * h )
         tr_wait( 200 );
     }
 
+    tr_lockFree( h->lock );
     free( h->tag );
     free( h );
     fprintf( stderr, "tr_close() completed.\n" );