Catch data-race.

CHANGELOG.md

@@ -11,6 +11,7 @@ Bug fixes
 * `NativeCallContext<'_>` (with a lifetime parameter) now parses correctly in the `#[export_module]`
   macro. This is to allow for `rust_2018_idioms` lints (thanks [`@ltabis`](https://github.com/ltabis) [864](https://github.com/rhaiscript/rhai/issues/864)).
 * The `sync` feature now works properly in `no-std` builds (thanks [`@misssonder`](https://github.com/misssonder) [874](https://github.com/rhaiscript/rhai/pull/874)).
+* More data-race conditions are caught and returned as errors instead of panicking.
 
 New features
 ------------

src/eval/chaining.rs

@@ -9,9 +9,9 @@ use crate::{
     calc_fn_hash, Dynamic, Engine, ExclusiveRange, FnArgsVec, InclusiveRange, OnceCell, Position,
     RhaiResult, RhaiResultOf, Scope, ERR,
 };
-use std::hash::Hash;
 #[cfg(feature = "no_std")]
 use std::prelude::v1::*;
+use std::{convert::TryInto, hash::Hash};
 
 /// Function call hashes to index getters and setters.
 static INDEXER_HASHES: OnceCell<(u64, u64)> = OnceCell::new();
@@ -146,7 +146,7 @@ impl Engine {
                     }
                 };
 
-                Ok(arr.get_mut(arr_idx).map(Target::from).unwrap())
+                arr.get_mut(arr_idx).unwrap().try_into()
             }
 
             #[cfg(not(feature = "no_index"))]
@@ -192,7 +192,7 @@ impl Engine {
                 }
 
                 if let Some(value) = map.get_mut(index.as_str()) {
-                    Ok(Target::from(value))
+                    value.try_into()
                 } else if self.fail_on_invalid_map_property() {
                     Err(ERR::ErrorPropertyNotFound(index.to_string(), idx_pos).into())
                 } else {
@@ -508,7 +508,7 @@ impl Engine {
                 this_ptr.map_or_else(
                     || Err(ERR::ErrorUnboundThis(*var_pos).into()),
                     |this_ptr| {
-                        let target = &mut this_ptr.into();
+                        let target = &mut this_ptr.try_into()?;
                         let scope = Some(scope);
                         self.eval_dot_index_chain_raw(
                             global, caches, scope, None, lhs, expr, target, rhs, idx_values,
@@ -969,7 +969,7 @@ impl Engine {
                                 })?;
 
                             {
-                                let orig_val = &mut (&mut orig_val).into();
+                                let orig_val = &mut (&mut orig_val).try_into()?;
 
                                 self.eval_op_assignment(
                                     global, caches, op_info, root, orig_val, new_val,
@@ -1139,7 +1139,7 @@ impl Engine {
                                         _ => Err(err),
                                     })?;
 
-                                let val = &mut (&mut val).into();
+                                let val = &mut (&mut val).try_into()?;
 
                                 let (result, may_be_changed) = self.eval_dot_index_chain_raw(
                                     global, caches, s, _this_ptr, root, rhs, val, &x.rhs,

src/eval/expr.rs

@@ -7,7 +7,7 @@ use crate::types::dynamic::AccessMode;
 use crate::{Dynamic, Engine, RhaiResult, RhaiResultOf, Scope, SmartString, ERR};
 #[cfg(feature = "no_std")]
 use std::prelude::v1::*;
-use std::{fmt::Write, num::NonZeroUsize};
+use std::{convert::TryInto, fmt::Write, num::NonZeroUsize};
 
 impl Engine {
     /// Search for a module within an imports stack.
@@ -142,7 +142,7 @@ impl Engine {
 
         let val = scope.get_mut_by_index(index);
 
-        Ok(val.into())
+        val.try_into()
     }
     /// Search for a variable within the scope or within imports,
     /// depending on whether the variable name is namespace-qualified.

src/eval/stmt.rs

@@ -8,9 +8,12 @@ use crate::func::{get_builtin_op_assignment_fn, get_hasher};
 use crate::tokenizer::Token;
 use crate::types::dynamic::{AccessMode, Union};
 use crate::{Dynamic, Engine, RhaiResult, RhaiResultOf, Scope, VarDefInfo, ERR, INT};
-use std::hash::{Hash, Hasher};
 #[cfg(feature = "no_std")]
 use std::prelude::v1::*;
+use std::{
+    convert::TryInto,
+    hash::{Hash, Hasher},
+};
 
 impl Engine {
     /// If the value is a string, intern it.
@@ -310,7 +313,7 @@ impl Engine {
 
                         self.track_operation(global, lhs.position())?;
 
-                        let target = &mut this_ptr.unwrap().into();
+                        let target = &mut this_ptr.unwrap().try_into()?;
 
                         self.eval_op_assignment(global, caches, op_info, lhs, target, rhs_val)?;
                     }

src/eval/target.rs

@@ -1,9 +1,12 @@
 //! Type to hold a mutable reference to the target of an evaluation.
 
-use crate::{Dynamic, Position, RhaiResultOf};
-use std::borrow::{Borrow, BorrowMut};
+use crate::{Dynamic, EvalAltResult, Position, RhaiError, RhaiResultOf};
 #[cfg(feature = "no_std")]
 use std::prelude::v1::*;
+use std::{
+    borrow::{Borrow, BorrowMut},
+    convert::TryFrom,
+};
 
 /// Calculate an offset+len pair given an actual length of the underlying array.
 ///
@@ -413,21 +416,25 @@ impl<'a> Target<'a> {
     }
 }
 
-impl<'a> From<&'a mut Dynamic> for Target<'a> {
+impl<'a> TryFrom<&'a mut Dynamic> for Target<'a> {
+    type Error = RhaiError;
+
     #[inline]
-    fn from(value: &'a mut Dynamic) -> Self {
+    fn try_from(value: &'a mut Dynamic) -> Result<Self, Self::Error> {
         #[cfg(not(feature = "no_closure"))]
         if value.is_shared() {
             // Cloning is cheap for a shared value
             let shared_value = value.clone();
-            let guard = value.write_lock::<Dynamic>().unwrap();
-            return Self::SharedValue {
+            let Some(guard) = value.write_lock::<Dynamic>() else {
+                return Err(EvalAltResult::ErrorDataRace(String::new(), Position::NONE).into());
+            };
+            return Ok(Self::SharedValue {
                 guard,
                 shared_value,
-            };
+            });
         }
 
-        Self::RefMut(value)
+        Ok(Self::RefMut(value))
     }
 }
 

src/types/error.rs

@@ -162,6 +162,7 @@ impl fmt::Display for EvalAltResult {
             Self::ErrorIndexNotFound(s, ..) => write!(f, "Invalid index: {s}")?,
             Self::ErrorFunctionNotFound(s, ..) => write!(f, "Function not found: {s}")?,
             Self::ErrorModuleNotFound(s, ..) => write!(f, "Module not found: {s}")?,
+            Self::ErrorDataRace(s, ..) if s.is_empty() => write!(f, "Data race detected")?,
             Self::ErrorDataRace(s, ..) => write!(f, "Data race detected on variable '{s}'")?,
 
             Self::ErrorDotExpr(s, ..) if s.is_empty() => f.write_str("Malformed dot expression")?,

tests/arrays.rs

@@ -1,6 +1,6 @@
 #![cfg(not(feature = "no_index"))]
 use rhai::{Array, Dynamic, Engine, EvalAltResult, ParseErrorType, Position, INT};
-use std::iter::FromIterator;
+use std::{convert::TryInto, iter::FromIterator};
 
 #[test]
 fn test_arrays() {
@@ -509,7 +509,7 @@ fn test_array_invalid_index_callback() {
     engine.on_invalid_array_index(|arr, index, _| match index {
         -100 => {
             arr.push((42 as INT).into());
-            Ok(arr.last_mut().unwrap().into())
+            arr.last_mut().unwrap().try_into()
         }
         100 => Ok(Dynamic::from(100 as INT).into()),
         _ => Err(EvalAltResult::ErrorArrayBounds(arr.len(), index, Position::NONE).into()),

tests/maps.rs

@@ -1,5 +1,6 @@
 #![cfg(not(feature = "no_object"))]
 use rhai::{Dynamic, Engine, EvalAltResult, Map, ParseErrorType, Position, Scope, INT};
+use std::convert::TryInto;
 
 #[test]
 fn test_map_indexing() {
@@ -264,7 +265,7 @@ fn test_map_missing_property_callback() {
     engine.on_map_missing_property(|map, prop, _| match prop {
         "x" => {
             map.insert("y".into(), (42 as INT).into());
-            Ok(map.get_mut("y").unwrap().into())
+            map.get_mut("y").unwrap().try_into()
         }
         "z" => Ok(Dynamic::from(100 as INT).into()),
         _ => Err(EvalAltResult::ErrorPropertyNotFound(prop.to_string(), Position::NONE).into()),