@@ -517,14 +517,16 @@ impl<T> Lock<T> {
517517 }
518518
519519 // The lock was successfully acquired.
520- Poll :: Ready ( LockGuard ( self . 0 . clone ( ) ) )
520+ Poll :: Ready ( LockGuard ( Some ( self . 0 . clone ( ) ) ) )
521521 }
522522}
523523
524524/// A lock guard.
525525///
526526/// When dropped, ownership of the inner value is returned back to the lock.
527- struct LockGuard < T > ( Arc < LockState < T > > ) ;
527+ /// The inner value is always Some, except when the lock is dropped, where we
528+ /// set it to None. See comment in drop().
529+ struct LockGuard < T > ( Option < Arc < LockState < T > > > ) ;
528530
529531unsafe impl < T : Send > Send for LockGuard < T > { }
530532unsafe impl < T : Sync > Sync for LockGuard < T > { }
@@ -534,7 +536,7 @@ impl<T> LockGuard<T> {
534536 ///
535537 /// When this lock guard gets dropped, all registered tasks will be woken up.
536538 fn register ( & self , cx : & Context < ' _ > ) {
537- let mut list = self . 0 . wakers . lock ( ) . unwrap ( ) ;
539+ let mut list = self . 0 . as_ref ( ) . unwrap ( ) . wakers . lock ( ) . unwrap ( ) ;
538540
539541 if list. iter ( ) . all ( |w| !w. will_wake ( cx. waker ( ) ) ) {
540542 list. push ( cx. waker ( ) . clone ( ) ) ;
@@ -544,11 +546,22 @@ impl<T> LockGuard<T> {
544546
545547impl < T > Drop for LockGuard < T > {
546548 fn drop ( & mut self ) {
549+ // Set the Option to None and take its value so we can drop the Arc
550+ // before we wake up the tasks.
551+ let lock = self . 0 . take ( ) . unwrap ( ) ;
552+ 553+ // Prepare to wake up all registered tasks interested in acquiring the lock.
554+ let wakers: Vec < _ > = lock. wakers . lock ( ) . unwrap ( ) . drain ( ..) . collect ( ) ;
555+ 547556 // Release the lock.
548- self . 0 . locked . store ( false , Ordering :: Release ) ;
557+ lock. locked . store ( false , Ordering :: Release ) ;
558+ 559+ // Drop the Arc _before_ waking up the tasks, to avoid races. See
560+ // reproducer and discussion in https://github.com/async-rs/async-std/issues/1001.
561+ drop ( lock) ;
549562
550563 // Wake up all registered tasks interested in acquiring the lock.
551- for w in self . 0 . wakers . lock ( ) . unwrap ( ) . drain ( .. ) {
564+ for w in wakers {
552565 w. wake ( ) ;
553566 }
554567 }
@@ -558,13 +571,19 @@ impl<T> Deref for LockGuard<T> {
558571 type Target = T ;
559572
560573 fn deref ( & self ) -> & T {
561- unsafe { & * self . 0 . value . get ( ) }
574+ // SAFETY: Safe because the lock is held when this method is called. And
575+ // the inner value is always Some since it is only set to None in
576+ // drop().
577+ unsafe { & * self . 0 . as_ref ( ) . unwrap ( ) . value . get ( ) }
562578 }
563579}
564580
565581impl < T > DerefMut for LockGuard < T > {
566582 fn deref_mut ( & mut self ) -> & mut T {
567- unsafe { & mut * self . 0 . value . get ( ) }
583+ // SAFETY: Safe because the lock is held when this method is called. And
584+ // the inner value is always Some since it is only set to None in
585+ // drop().
586+ unsafe { & mut * self . 0 . as_ref ( ) . unwrap ( ) . value . get ( ) }
568587 }
569588}
570589
0 commit comments