Ignore:
Timestamp:
May 10, 2023, 2:46:05 PM (19 months ago)
Author:
caparsons <caparson@…>
Branches:
ADT, ast-experimental, master
Children:
8fd1b7c
Parents:
e50fce1
Message:

Added fix for cond var timeout handling race. Cleanup of locks.hfa/cfa changes is an ongoing TODO

Location:
libcfa/src/concurrency
Files:
2 edited

Legend:

Unmodified
Added
Removed

  • libcfa/src/concurrency/locks.cfa

    re50fce1 rfece3d9  
    171171}
    172172
    173 size_t on_wait( blocking_lock & this ) with( this ) {
     173size_t on_wait( blocking_lock & this, __cfa_pre_park pp_fn, void * pp_datum ) with( this ) {
    174174        lock( lock __cfaabi_dbg_ctx2 );
    175175        /* paranoid */ verifyf( owner != 0p, "Attempt to release lock %p that isn't held", &this );
    … …  
    184184        unlock( lock );
    185185
    186     park();
     186    pre_park_then_park( pp_fn, pp_datum );
    187187
    188188        return ret;
    … …  
    396396        }
    397397
    398     static size_t block_and_get_recursion( info_thread(L) & i ) {
     398    static size_t block_and_get_recursion( info_thread(L) & i, __cfa_pre_park pp_fn, void * pp_datum ) {
    399399        size_t recursion_count = 0;
    400400                if ( i.lock ) {
    401401                        // if lock was passed get recursion count to reset to after waking thread
    402                         recursion_count = on_wait( *i.lock ); // this call blocks
    403                 } else park( );
     402                        recursion_count = on_wait( *i.lock, pp_fn, pp_datum ); // this call blocks
     403                } else pre_park_then_park( pp_fn, pp_datum );
    404404        return recursion_count;
    405405    }
     406    static size_t block_and_get_recursion( info_thread(L) & i ) { return block_and_get_recursion( i, pre_park_noop, 0p ); }
    406407
    407408        // helper for wait()'s' with no timeout
    … …  
    424425                queue_info_thread( this, i );
    425426
     427    static void cond_alarm_register( void * node_ptr ) { register_self( (alarm_node_t *)node_ptr ); }
     428
    426429        // helper for wait()'s' with a timeout
    427430        static void queue_info_thread_timeout( condition_variable(L) & this, info_thread(L) & info, Duration t, Alarm_Callback callback ) with(this) {
    … …  
    433436
    434437                // registers alarm outside cond lock to avoid deadlock
    435                 register_self( &node_wrap.alarm_node );
     438                // register_self( &node_wrap.alarm_node );
    436439
    437440                // blocks here
    438         size_t recursion_count = block_and_get_recursion( info );
     441        size_t recursion_count = block_and_get_recursion( info, cond_alarm_register, (void *)(&node_wrap.alarm_node) );
    439442                // park();
    440443
    … …  
    503506                info_thread( L ) i = { active_thread(), info, &l };
    504507                insert_last( blocked_threads, i );
    505                 size_t recursion_count = on_wait( *i.lock ); // blocks here
     508                size_t recursion_count = on_wait( *i.lock, pre_park_noop, 0p ); // blocks here
    506509                // park( );
    507510                on_wakeup(*i.lock, recursion_count);
    … …  
    552555        //      return recursion_count;
    553556        // }
    554 
    555557       
    556558        static void queue_info_thread_timeout( pthread_cond_var(L) & this, info_thread(L) & info, Duration t, Alarm_Callback callback ) with(this) {
    … …  
    562564
    563565                // registers alarm outside cond lock to avoid deadlock
    564                 register_self( &node_wrap.alarm_node );
     566                // register_self( &node_wrap.alarm_node ); // C_TODO: fix race: registers itself and then alarm handler calls on_notify before block_and_get_recursion is run
    565567
    566568                // blocks here
    567         size_t recursion_count = block_and_get_recursion( info );
     569        size_t recursion_count = block_and_get_recursion( info, cond_alarm_register, (void *)(&node_wrap.alarm_node) );
    568570                // park();
    569571

  • libcfa/src/concurrency/locks.hfa

    re50fce1 rfece3d9  
    4444// - change messy big blocking lock from inheritance to composition to remove need for flags
    4545
     46typedef void (*__cfa_pre_park)( void * );
     47
     48static inline void pre_park_noop( void * ) {}
     49
     50//-----------------------------------------------------------------------------
     51// is_blocking_lock
     52forall( L & | sized(L) )
     53trait is_blocking_lock {
     54        // For synchronization locks to use when acquiring
     55        void on_notify( L &, struct thread$ * );
     56
     57        // For synchronization locks to use when releasing
     58        size_t on_wait( L &, __cfa_pre_park pp_fn, void * pp_datum );
     59
     60        // to set recursion count after getting signalled;
     61        void on_wakeup( L &, size_t recursion );
     62};
     63
     64static inline void pre_park_then_park( __cfa_pre_park pp_fn, void * pp_datum ) {
     65    pp_fn( pp_datum );
     66    park();
     67}
     68
    4669//-----------------------------------------------------------------------------
    4770// Semaphore
    … …  
    6992static inline bool   try_lock ( single_acquisition_lock & this ) { return try_lock( (blocking_lock &)this ); }
    7093static inline void   unlock   ( single_acquisition_lock & this ) { unlock  ( (blocking_lock &)this ); }
    71 static inline size_t on_wait  ( single_acquisition_lock & this ) { return on_wait ( (blocking_lock &)this ); }
     94static inline size_t on_wait  ( single_acquisition_lock & this, __cfa_pre_park pp_fn, void * pp_datum ) { return on_wait ( (blocking_lock &)this, pp_fn, pp_datum ); }
    7295static inline void   on_wakeup( single_acquisition_lock & this, size_t v ) { on_wakeup ( (blocking_lock &)this, v ); }
    7396static inline void   on_notify( single_acquisition_lock & this, struct thread$ * t ) { on_notify( (blocking_lock &)this, t ); }
    … …  
    86109static inline bool   try_lock ( owner_lock & this ) { return try_lock( (blocking_lock &)this ); }
    87110static inline void   unlock   ( owner_lock & this ) { unlock  ( (blocking_lock &)this ); }
    88 static inline size_t on_wait  ( owner_lock & this ) { return on_wait ( (blocking_lock &)this ); }
     111static inline size_t on_wait  ( owner_lock & this, __cfa_pre_park pp_fn, void * pp_datum ) { return on_wait ( (blocking_lock &)this, pp_fn, pp_datum ); }
    89112static inline void   on_wakeup( owner_lock & this, size_t v ) { on_wakeup ( (blocking_lock &)this, v ); }
    90113static inline void   on_notify( owner_lock & this, struct thread$ * t ) { on_notify( (blocking_lock &)this, t ); }
    … …  
    218241
    219242static inline void on_notify( futex_mutex & f, thread$ * t){ unpark(t); }
    220 static inline size_t on_wait( futex_mutex & f ) { unlock(f); park(); return 0; }
     243static inline size_t on_wait( futex_mutex & this, __cfa_pre_park pp_fn, void * pp_datum ) {
     244    unlock( this );
     245    pre_park_then_park( pp_fn, pp_datum );
     246    return 0;
     247}
    221248
    222249// to set recursion count after getting signalled;
    … …  
    288315
    289316static inline void on_notify( go_mutex & f, thread$ * t){ unpark( t ); }
    290 static inline size_t on_wait( go_mutex & f ) { unlock( f ); park(); return 0; }
     317static inline size_t on_wait( go_mutex & this, __cfa_pre_park pp_fn, void * pp_datum ) {
     318    unlock( this );
     319    pre_park_then_park( pp_fn, pp_datum );
     320    return 0;
     321}
    291322static inline void on_wakeup( go_mutex & f, size_t recursion ) {}
    292323
    … …  
    362393
    363394static inline void on_notify( exp_backoff_then_block_lock & this, struct thread$ * t ) { unpark( t ); }
    364 static inline size_t on_wait( exp_backoff_then_block_lock & this ) { unlock( this ); park(); return 0; }
     395static inline size_t on_wait( exp_backoff_then_block_lock & this, __cfa_pre_park pp_fn, void * pp_datum ) {
     396    unlock( this );
     397    pre_park_then_park( pp_fn, pp_datum );
     398    return 0;
     399}
    365400static inline void on_wakeup( exp_backoff_then_block_lock & this, size_t recursion ) { lock( this ); }
    366401
    … …  
    419454    unlock( lock );
    420455}
    421 static inline size_t on_wait( fast_block_lock & this) { unlock(this); park(); return 0; }
     456static inline size_t on_wait( fast_block_lock & this, __cfa_pre_park pp_fn, void * pp_datum ) {
     457    unlock( this );
     458    pre_park_then_park( pp_fn, pp_datum );
     459    return 0;
     460}
    422461static inline void on_wakeup( fast_block_lock & this, size_t recursion ) { }
    423462
    … …  
    497536}
    498537
    499 static inline void on_notify(simple_owner_lock & this, thread$ * t ) with(this) {
     538static inline void on_notify( simple_owner_lock & this, thread$ * t ) with(this) {
    500539        lock( lock __cfaabi_dbg_ctx2 );
    501540        // lock held
    … …  
    512551}
    513552
    514 static inline size_t on_wait( simple_owner_lock & this ) with(this) {
     553static inline size_t on_wait( simple_owner_lock & this, __cfa_pre_park pp_fn, void * pp_datum ) with(this) {
    515554        lock( lock __cfaabi_dbg_ctx2 );
    516555        /* paranoid */ verifyf( owner != 0p, "Attempt to release lock %p that isn't held", &this );
    … …  
    524563    active_thread()->link_node = (void *)&node;
    525564        unlock( lock );
    526     park();
     565
     566    pre_park_then_park( pp_fn, pp_datum );
    527567
    528568        return ret;
    … …  
    624664        unpark(t);
    625665}
    626 static inline size_t on_wait( spin_queue_lock & this ) { unlock( this ); park(); return 0; }
     666static inline size_t on_wait( spin_queue_lock & this, __cfa_pre_park pp_fn, void * pp_datum ) {
     667    unlock( this );
     668    pre_park_then_park( pp_fn, pp_datum );
     669    return 0;
     670}
    627671static inline void on_wakeup( spin_queue_lock & this, size_t recursion ) { lock( this ); }
    628672
    … …  
    665709
    666710static inline void on_notify( mcs_block_spin_lock & this, struct thread$ * t ) { unpark( t ); }
    667 static inline size_t on_wait( mcs_block_spin_lock & this) { unlock( this ); park(); return 0; }
     711static inline size_t on_wait( mcs_block_spin_lock & this, __cfa_pre_park pp_fn, void * pp_datum ) {
     712    unlock( this );
     713    pre_park_then_park( pp_fn, pp_datum );
     714    return 0;
     715}
    668716static inline void on_wakeup( mcs_block_spin_lock & this, size_t recursion ) {lock( this ); }
    669717
    … …  
    717765        unpark(t);
    718766}
    719 static inline size_t on_wait( block_spin_lock & this ) { unlock( this ); park(); return 0; }
     767static inline size_t on_wait( block_spin_lock & this, __cfa_pre_park pp_fn, void * pp_datum ) {
     768    unlock( this );
     769    pre_park_then_park( pp_fn, pp_datum );
     770    return 0;
     771}
    720772static inline void on_wakeup( block_spin_lock & this, size_t recursion ) with(this) {
    721773        // now we acquire the entire block_spin_lock upon waking up
    … …  
    724776        unlock( lock ); // Now we release the internal fast_spin_lock
    725777}
    726 
    727 //-----------------------------------------------------------------------------
    728 // is_blocking_lock
    729 forall( L & | sized(L) )
    730 trait is_blocking_lock {
    731         // For synchronization locks to use when acquiring
    732         void on_notify( L &, struct thread$ * );
    733 
    734         // For synchronization locks to use when releasing
    735         size_t on_wait( L & );
    736 
    737         // to set recursion count after getting signalled;
    738         void on_wakeup( L &, size_t recursion );
    739 };
    740778
    741779//-----------------------------------------------------------------------------
Note: See TracChangeset for help on using the changeset viewer.