Prevent data races.

src/any.rs

@@ -282,6 +282,11 @@ impl Dynamic {
     }
 
     /// Get the TypeId of the value held by this `Dynamic`.
+    ///
+    /// # Panics and Deadlocks When Value is Shared
+    ///
+    /// Under the `sync` feature, this call may deadlock.
+    /// Otherwise, this call panics if the data is currently borrowed for write.
     pub fn type_id(&self) -> TypeId {
         match &self.0 {
             Union::Unit(_) => TypeId::of::<()>(),
@@ -307,6 +312,11 @@ impl Dynamic {
     }
 
     /// Get the name of the type of the value held by this `Dynamic`.
+    ///
+    /// # Panics and Deadlocks When Value is Shared
+    ///
+    /// Under the `sync` feature, this call may deadlock.
+    /// Otherwise, this call panics if the data is currently borrowed for write.
     pub fn type_name(&self) -> &'static str {
         match &self.0 {
             Union::Unit(_) => "()",
@@ -752,6 +762,28 @@ impl Dynamic {
         }
     }
 
+    /// Is the `Dynamic` a shared value that is locked?
+    ///
+    /// ## Note
+    ///
+    /// Under the `sync` feature, shared values use `RwLock` and they are never locked.
+    /// Access just waits until the `RwLock` is released.
+    /// So this method always returns `false` under `sync`.
+    #[inline(always)]
+    pub fn is_locked(&self) -> bool {
+        match self.0 {
+            #[cfg(not(feature = "no_shared"))]
+            Union::Shared(ref _cell) => {
+                #[cfg(not(feature = "sync"))]
+                return _cell.try_borrow().is_err();
+
+                #[cfg(feature = "sync")]
+                return false;
+            }
+            _ => false,
+        }
+    }
+
     /// Get a reference of a specific type to the `Dynamic`.
     /// Casting to `Dynamic` just returns a reference to it.
     ///

src/api.rs

@@ -3,6 +3,7 @@
 use crate::any::{Dynamic, Variant};
 use crate::engine::{Engine, Imports, State};
 use crate::error::ParseError;
+use crate::fn_call::ensure_no_data_race;
 use crate::fn_native::{IteratorFn, SendSync};
 use crate::module::{FuncReturn, Module};
 use crate::optimize::{optimize_into_ast, OptimizationLevel};
@@ -1282,6 +1283,11 @@ impl Engine {
         let mut mods = Imports::new();
         let args = args.as_mut();
 
+        // Check for data race.
+        if cfg!(not(feature = "no_shared")) {
+            ensure_no_data_race(name, args, false)?;
+        }
+
         self.call_script_fn(
             scope, &mut mods, &mut state, lib, this_ptr, name, fn_def, args, 0,
         )

src/engine.rs

@@ -41,14 +41,11 @@ use crate::stdlib::{
     collections::{HashMap, HashSet},
     fmt, format,
     iter::{empty, once},
+    ops::DerefMut,
     string::{String, ToString},
     vec::Vec,
 };
 
-#[cfg(not(feature = "no_shared"))]
-#[cfg(not(feature = "no_object"))]
-use crate::stdlib::ops::DerefMut;
-
 #[cfg(not(feature = "no_index"))]
 use crate::stdlib::any::TypeId;
 
@@ -561,7 +558,8 @@ pub fn search_imports_mut<'s>(
     })
 }
 
-/// Search for a variable within the scope and imports
+/// Search for a variable within the scope or within imports,
+/// depending on whether the variable name is qualified.
 pub fn search_namespace<'s, 'a>(
     scope: &'s mut Scope,
     mods: &'s mut Imports,
@@ -631,6 +629,13 @@ pub fn search_scope_only<'s, 'a>(
     };
 
     let (val, typ) = scope.get_mut(index);
+
+    // Check for data race - probably not necessary because the only place it should conflict is in a method call
+    //                       when the object variable is also used as a parameter.
+    // if cfg!(not(feature = "no_shared")) && val.is_locked() {
+    //     return Err(Box::new(EvalAltResult::ErrorDataRace(name.into(), *pos)));
+    // }
+
     Ok((val, name, typ, *pos))
 }
 

src/fn_call.rs

@@ -160,6 +160,31 @@ fn add_captured_variables_into_scope<'s>(
         );
 }
 
+#[inline(always)]
+pub fn ensure_no_data_race(
+    fn_name: &str,
+    args: &FnCallArgs,
+    is_ref: bool,
+) -> Result<(), Box<EvalAltResult>> {
+    if cfg!(not(feature = "no_shared")) {
+        let skip = if is_ref { 1 } else { 0 };
+
+        if let Some((n, _)) = args
+            .iter()
+            .skip(skip)
+            .enumerate()
+            .find(|(_, a)| a.is_locked())
+        {
+            return Err(Box::new(EvalAltResult::ErrorDataRace(
+                format!("argument #{} of function '{}'", n + 1 + skip, fn_name),
+                Position::none(),
+            )));
+        }
+    }
+
+    Ok(())
+}
+
 impl Engine {
     /// Call a native Rust function registered with the `Engine`.
     /// Position in `EvalAltResult` is `None` and must be set afterwards.
@@ -430,6 +455,11 @@ impl Engine {
         def_val: Option<bool>,
         level: usize,
     ) -> Result<(Dynamic, bool), Box<EvalAltResult>> {
+        // Check for data race.
+        if cfg!(not(feature = "no_shared")) {
+            ensure_no_data_race(fn_name, args, is_ref)?;
+        }
+
         // Qualifiers (none) + function name + number of arguments + argument `TypeId`'s.
         let arg_types = args.iter().map(|a| a.type_id());
         let hash_fn = calc_fn_hash(empty(), fn_name, args.len(), arg_types);

src/result.rs

@@ -69,6 +69,8 @@ pub enum EvalAltResult {
     ErrorVariableNotFound(String, Position),
     /// Usage of an unknown module. Wrapped value is the name of the module.
     ErrorModuleNotFound(String, Position),
+    /// Data race detected when accessing a variable. Wrapped value is the name of the variable.
+    ErrorDataRace(String, Position),
     /// Assignment to an inappropriate LHS (left-hand-side) expression.
     ErrorAssignmentToUnknownLHS(Position),
     /// Assignment to a constant variable.
@@ -136,7 +138,8 @@ impl EvalAltResult {
             Self::ErrorLogicGuard(_) => "Boolean value expected",
             Self::ErrorFor(_) => "For loop expects an array, object map, or range",
             Self::ErrorVariableNotFound(_, _) => "Variable not found",
-            Self::ErrorModuleNotFound(_, _) => "module not found",
+            Self::ErrorModuleNotFound(_, _) => "Module not found",
+            Self::ErrorDataRace(_, _) => "Data race detected when accessing variable",
             Self::ErrorAssignmentToUnknownLHS(_) => {
                 "Assignment to an unsupported left-hand side expression"
             }
@@ -180,6 +183,7 @@ impl fmt::Display for EvalAltResult {
 
             Self::ErrorFunctionNotFound(s, _)
             | Self::ErrorVariableNotFound(s, _)
+            | Self::ErrorDataRace(s, _)
             | Self::ErrorModuleNotFound(s, _) => write!(f, "{}: '{}'", desc, s)?,
 
             Self::ErrorDotExpr(s, _) if !s.is_empty() => write!(f, "{}", s)?,
@@ -289,6 +293,7 @@ impl EvalAltResult {
             | Self::ErrorFor(pos)
             | Self::ErrorVariableNotFound(_, pos)
             | Self::ErrorModuleNotFound(_, pos)
+            | Self::ErrorDataRace(_, pos)
             | Self::ErrorAssignmentToUnknownLHS(pos)
             | Self::ErrorAssignmentToConstant(_, pos)
             | Self::ErrorMismatchOutputType(_, _, pos)
@@ -329,6 +334,7 @@ impl EvalAltResult {
             | Self::ErrorFor(pos)
             | Self::ErrorVariableNotFound(_, pos)
             | Self::ErrorModuleNotFound(_, pos)
+            | Self::ErrorDataRace(_, pos)
             | Self::ErrorAssignmentToUnknownLHS(pos)
             | Self::ErrorAssignmentToConstant(_, pos)
             | Self::ErrorMismatchOutputType(_, _, pos)

tests/shared.rs

@@ -283,3 +283,42 @@ fn test_shared() -> Result<(), Box<EvalAltResult>> {
 
     Ok(())
 }
+
+#[test]
+fn test_shared_data_race() -> Result<(), Box<EvalAltResult>> {
+    let engine = Engine::new();
+
+    #[cfg(not(feature = "no_function"))]
+    #[cfg(not(feature = "no_object"))]
+    {
+        assert_eq!(
+            engine.eval::<INT>(
+                r#"
+                    fn foo(x) { this += x };
+
+                    let a = shared(41);
+                    a.foo(1);
+                    a
+                "#
+            )?,
+            42
+        );
+
+        assert!(matches!(
+            *engine
+                .eval::<INT>(
+                    r#"
+                        fn foo(x) { this += x };
+
+                        let a = shared(42);
+                        a.foo(a);
+                        a
+                    "#
+                )
+                .expect_err("should error"),
+            EvalAltResult::ErrorDataRace(_, _)
+        ));
+    }
+
+    Ok(())
+}