From e804f88d60071f0d89db85aaa4a073857904b545 Mon Sep 17 00:00:00 2001
From: Zahari Dichev <zaharidichev@gmail.com>
Date: Fri, 23 Oct 2020 20:07:00 +0300
Subject: sync: add mem::forget to RwLockWriteGuard::downgrade. (#2957)

Currently when `RwLockWriteGuard::downgrade` the `MAX_READS - 1`
permits are added to the semaphore. When `RwLockWriteGuard::drop`
gets invoked however another `MAX_READS` permits are added. This
results in releasing more permits that were actually aquired when
downgrading a write to a read lock. This is why we need to call
`mem::forget` on the `RwLockWriteGuard` in order to avoid
invoking the destructor.

Fixes: #2941
---
 tokio/src/sync/rwlock.rs            |  5 ++---
 tokio/src/sync/tests/loom_rwlock.rs | 25 +++++++++++++++++++++++--
 tokio/src/sync/tests/mod.rs         |  1 +
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/tokio/src/sync/rwlock.rs b/tokio/src/sync/rwlock.rs
index a84c4c12..750765fb 100644
--- a/tokio/src/sync/rwlock.rs
+++ b/tokio/src/sync/rwlock.rs
@@ -375,8 +375,6 @@ impl<'a, T: ?Sized> RwLockWriteGuard<'a, T> {
     /// let n = n.downgrade();
     /// assert_eq!(*n, 1, "downgrade is atomic");
     ///
-    /// assert_eq!(*lock.read().await, 1, "additional readers can obtain locks");
-    ///
     /// drop(n);
     /// handle.await.unwrap();
     /// assert_eq!(*lock.read().await, 2, "second writer obtained write lock");
@@ -389,7 +387,8 @@ impl<'a, T: ?Sized> RwLockWriteGuard<'a, T> {
 
         // Release all but one of the permits held by the write guard
         s.release(MAX_READS - 1);
-
+        // NB: Forget to avoid drop impl from being called.
+        mem::forget(self);
         RwLockReadGuard {
             s,
             data,
diff --git a/tokio/src/sync/tests/loom_rwlock.rs b/tokio/src/sync/tests/loom_rwlock.rs
index 48d06e1d..2834a263 100644
--- a/tokio/src/sync/tests/loom_rwlock.rs
+++ b/tokio/src/sync/tests/loom_rwlock.rs
@@ -6,7 +6,7 @@ use std::sync::Arc;
 
 #[test]
 fn concurrent_write() {
-    let mut b = loom::model::Builder::new();
+    let b = loom::model::Builder::new();
 
     b.check(|| {
         let rwlock = Arc::new(RwLock::<u32>::new(0));
@@ -37,7 +37,7 @@ fn concurrent_write() {
 
 #[test]
 fn concurrent_read_write() {
-    let mut b = loom::model::Builder::new();
+    let b = loom::model::Builder::new();
 
     b.check(|| {
         let rwlock = Arc::new(RwLock::<u32>::new(0));
@@ -76,3 +76,24 @@ fn concurrent_read_write() {
         assert_eq!(10, *guard);
     });
 }
+#[test]
+fn downgrade() {
+    loom::model(|| {
+        let lock = Arc::new(RwLock::new(1));
+
+        let n = block_on(lock.write());
+
+        let cloned_lock = lock.clone();
+        let handle = thread::spawn(move || {
+            let mut guard = block_on(cloned_lock.write());
+            *guard = 2;
+        });
+
+        let n = n.downgrade();
+        assert_eq!(*n, 1);
+
+        drop(n);
+        handle.join().unwrap();
+        assert_eq!(*block_on(lock.read()), 2);
+    });
+}
diff --git a/tokio/src/sync/tests/mod.rs b/tokio/src/sync/tests/mod.rs
index a78be6f3..c5d56019 100644
--- a/tokio/src/sync/tests/mod.rs
+++ b/tokio/src/sync/tests/mod.rs
@@ -12,4 +12,5 @@ cfg_loom! {
     mod loom_oneshot;
     mod loom_semaphore_batch;
     mod loom_watch;
+    mod loom_rwlock;
 }
-- 
cgit v1.2.3