Pārlūkot izejas kodu

Fix dead lock issues

Cesar Rodas 1 gadu atpakaļ
vecāks
revīzija
14d3469088
3 mainītis faili ar 86 papildinājumiem un 59 dzēšanām
  1. 52 30
      src/cmd/list.rs
  2. 34 28
      src/cmd/set.rs
  3. 0 1
      src/db/entry.rs

+ 52 - 30
src/cmd/list.rs

@@ -429,15 +429,57 @@ pub async fn lmove(conn: &Connection, mut args: VecDeque<Bytes>) -> Result<Value
     db.lock_keys(&to_lock);
 
     let mut to_create = None;
+    let is_same_key = source == destination;
 
     let result = db
         .get(&source)
         .map_mut(|v| match v {
-            Value::List(source) => conn
-                .db()
-                .get(&destination)
-                .map_mut(|v| match v {
-                    Value::List(target) => {
+            Value::List(source) => {
+                if is_same_key {
+                    // take a different approach to avoid a deadlock
+                    let element = if source_is_left {
+                        source.pop_front()
+                    } else {
+                        source.pop_back()
+                    };
+                    return if let Some(element) = element {
+                        let ret = element.clone_value();
+                        if target_is_left {
+                            source.push_front(element);
+                        } else {
+                            source.push_back(element);
+                        }
+                        Ok(ret)
+                    } else {
+                        Ok(Value::Null)
+                    };
+                }
+
+                conn.db()
+                    .get(&destination)
+                    .map_mut(|v| match v {
+                        Value::List(target) => {
+                            let element = if source_is_left {
+                                source.pop_front()
+                            } else {
+                                source.pop_back()
+                            };
+
+                            if let Some(element) = element {
+                                let ret = element.clone_value();
+                                if target_is_left {
+                                    target.push_front(element);
+                                } else {
+                                    target.push_back(element);
+                                }
+                                Ok(ret)
+                            } else {
+                                Ok(Value::Null)
+                            }
+                        }
+                        _ => Err(Error::WrongType),
+                    })
+                    .unwrap_or_else(|| {
                         let element = if source_is_left {
                             source.pop_front()
                         } else {
@@ -446,35 +488,15 @@ pub async fn lmove(conn: &Connection, mut args: VecDeque<Bytes>) -> Result<Value
 
                         if let Some(element) = element {
                             let ret = element.clone_value();
-                            if target_is_left {
-                                target.push_front(element);
-                            } else {
-                                target.push_back(element);
-                            }
+                            let mut h = VecDeque::new();
+                            h.push_front(element);
+                            to_create = Some(h);
                             Ok(ret)
                         } else {
                             Ok(Value::Null)
                         }
-                    }
-                    _ => Err(Error::WrongType),
-                })
-                .unwrap_or_else(|| {
-                    let element = if source_is_left {
-                        source.pop_front()
-                    } else {
-                        source.pop_back()
-                    };
-
-                    if let Some(element) = element {
-                        let ret = element.clone_value();
-                        let mut h = VecDeque::new();
-                        h.push_front(element);
-                        to_create = Some(h);
-                        Ok(ret)
-                    } else {
-                        Ok(Value::Null)
-                    }
-                }),
+                    })
+            }
             _ => Err(Error::WrongType),
         })
         .unwrap_or(Ok(Value::Null));

+ 34 - 28
src/cmd/set.rs

@@ -312,45 +312,51 @@ pub async fn smove(conn: &Connection, mut args: VecDeque<Bytes>) -> Result<Value
     let source = args.pop_front().ok_or(Error::Syntax)?;
     let destination = args.pop_front().ok_or(Error::Syntax)?;
     let member = args.pop_front().ok_or(Error::Syntax)?;
+    let mut to_insert = None;
     let result = conn
         .db()
         .get(&source)
         .map_mut(|v| match v {
-            Value::Set(set1) => conn
-                .db()
-                .get(&destination)
-                .map_mut(|v| match v {
-                    Value::Set(set2) => {
-                        if !set1.contains(&member) {
-                            return Ok(0.into());
-                        }
+            Value::Set(set1) => {
+                if source == destination {
+                    return Ok(if set1.contains(&member) { 1 } else { 0 }.into());
+                }
 
-                        if source == destination {
-                            return Ok(1.into());
-                        }
+                conn.db()
+                    .get(&destination)
+                    .map_mut(|v| match v {
+                        Value::Set(set2) => {
+                            if !set1.contains(&member) {
+                                return Ok(0.into());
+                            }
 
-                        set1.remove(&member);
-                        if set2.insert(member.clone()) {
-                            Ok(1.into())
-                        } else {
-                            Ok(0.into())
+                            set1.remove(&member);
+                            if set2.insert(member.clone()) {
+                                Ok(1.into())
+                            } else {
+                                conn.db().bump_version(&source);
+                                Ok(0.into())
+                            }
                         }
-                    }
-                    _ => Err(Error::WrongType),
-                })
-                .unwrap_or_else(|| {
-                    set1.remove(&member);
-                    #[allow(clippy::mutable_key_type)]
-                    let mut x = HashSet::new();
-                    x.insert(member.clone());
-                    conn.db().set(destination.clone(), x.into(), None);
-                    Ok(1.into())
-                }),
+                        _ => Err(Error::WrongType),
+                    })
+                    .unwrap_or_else(|| {
+                        set1.remove(&member);
+                        let mut x = HashSet::new();
+                        x.insert(member.clone());
+                        to_insert = Some(x);
+                        Ok(1.into())
+                    })
+            }
             _ => Err(Error::WrongType),
         })
         .unwrap_or(Ok(0.into()))?;
 
-    if result == Value::Integer(1) {
+    if let Some(x) = to_insert {
+        conn.db().set(destination.clone(), x.into(), None);
+    }
+
+    if let Value::Integer(1) = result {
         conn.db().bump_version(&source);
         conn.db().bump_version(&destination);
     }

+ 0 - 1
src/db/entry.rs

@@ -82,7 +82,6 @@ impl Entry {
     }
 
     pub fn get_mut(&self) -> RwLockWriteGuard<'_, Value> {
-        self.bump_version();
         self.value.write()
     }