Merge pull request #1 from paritytech/a-thread-safe-api

A thread safe api

Author: gnunicorn

Cargo.lock

@@ -158,7 +158,7 @@ impl WrappedCallable for WasmtimeFn {
         // Get the trampoline to call for this function.
         let exec_code_buf = self
             .store
-            .compiler_mut()
+            .compiler()
             .get_published_trampoline(&signature, value_size)
             .map_err(|e| Trap::new(format!("trampoline error: {:?}", e)))?;
 

crates/api/src/callable.rs

@@ -250,7 +250,7 @@ impl Module {
 
     unsafe fn compile(store: &Store, binary: &[u8]) -> Result<Self> {
         let compiled = CompiledModule::new(
-            &mut store.compiler_mut(),
+            store.compiler(),
             binary,
             store.engine().config().debug_info,
             store.engine().config().profiler.as_ref(),

crates/api/src/module.rs

@@ -1,8 +1,6 @@
 use anyhow::Result;
-use std::cell::RefCell;
 use std::fmt;
 use std::path::Path;
-use std::rc::Rc;
 use std::sync::{Arc, Mutex};
 use wasmparser::{OperatorValidatorConfig, ValidatingParserConfig};
 use wasmtime_environ::settings::{self, Configurable};
@@ -447,12 +445,12 @@ impl Engine {
 #[derive(Clone)]
 pub struct Store {
     // FIXME(#777) should be `Arc` and this type should be thread-safe
-    inner: Rc<StoreInner>,
+    inner: Arc<StoreInner>,
 }
 
 struct StoreInner {
     engine: Engine,
-    compiler: RefCell<Compiler>,
+    compiler: Compiler,
 }
 
 impl Store {
@@ -465,9 +463,9 @@ impl Store {
             engine.config.cache_config.clone(),
         );
         Store {
-            inner: Rc::new(StoreInner {
+            inner: Arc::new(StoreInner {
                 engine: engine.clone(),
-                compiler: RefCell::new(compiler),
+                compiler,
             }),
         }
     }
@@ -477,12 +475,8 @@ impl Store {
         &self.inner.engine
     }
 
-    pub(crate) fn compiler(&self) -> std::cell::Ref<'_, Compiler> {
-        self.inner.compiler.borrow()
-    }
-
-    pub(crate) fn compiler_mut(&self) -> std::cell::RefMut<'_, Compiler> {
-        self.inner.compiler.borrow_mut()
+    pub(crate) fn compiler(&self) -> &Compiler {
+        &self.inner.compiler
     }
 
     /// Returns whether the stores `a` and `b` refer to the same underlying
@@ -492,7 +486,7 @@ impl Store {
     /// to the same underlying storage, and this method can be used to determine
     /// whether two stores are indeed the same.
     pub fn same(a: &Store, b: &Store) -> bool {
-        Rc::ptr_eq(&a.inner, &b.inner)
+        Arc::ptr_eq(&a.inner, &b.inner)
     }
 }
 

crates/api/src/runtime.rs

@@ -13,7 +13,7 @@ use wasmtime_runtime::{Imports, InstanceHandle, VMFunctionBody};
 pub(crate) fn create_handle(
     module: Module,
     store: &Store,
-    finished_functions: PrimaryMap<DefinedFuncIndex, *mut [VMFunctionBody]>,
+    finished_functions: PrimaryMap<DefinedFuncIndex, *const [VMFunctionBody]>,
     state: Box<dyn Any>,
 ) -> Result<InstanceHandle> {
     let imports = Imports::new(

crates/api/src/trampoline/create_handle.rs

@@ -264,7 +264,7 @@ pub fn create_handle_with_function(
 
     let mut fn_builder_ctx = FunctionBuilderContext::new();
     let mut module = Module::new();
-    let mut finished_functions: PrimaryMap<DefinedFuncIndex, *mut [VMFunctionBody]> =
+    let mut finished_functions: PrimaryMap<DefinedFuncIndex, *const [VMFunctionBody]> =
         PrimaryMap::new();
     let mut code_memory = CodeMemory::new();
 
@@ -296,7 +296,7 @@ pub fn create_handle_with_function(
 
 pub unsafe fn create_handle_with_raw_function(
     ft: &FuncType,
-    func: *mut [VMFunctionBody],
+    func: *const [VMFunctionBody],
     store: &Store,
     state: Box<dyn Any>,
 ) -> Result<InstanceHandle> {

crates/api/src/trampoline/func.rs

@@ -11,6 +11,10 @@ use cranelift_frontend::{FunctionBuilder, FunctionBuilderContext};
 use cranelift_wasm::ModuleTranslationState;
 use std::collections::HashMap;
 use std::convert::TryFrom;
+use std::sync::{
+    atomic::{AtomicPtr, Ordering},
+    RwLock,
+};
 use wasmtime_debug::{emit_debugsections_image, DebugInfoData};
 use wasmtime_environ::entity::{EntityRef, PrimaryMap};
 use wasmtime_environ::isa::{TargetFrontendConfig, TargetIsa};
@@ -51,13 +55,16 @@ pub enum CompilationStrategy {
 pub struct Compiler {
     isa: Box<dyn TargetIsa>,
 
-    code_memory: CodeMemory,
     trap_registry: TrapRegistry,
-    trampoline_park: HashMap<VMSharedSignatureIndex, *const VMFunctionBody>,
     signatures: SignatureRegistry,
     strategy: CompilationStrategy,
     cache_config: CacheConfig,
+    inner: RwLock<CompilerInner>,
+}
 
+struct CompilerInner {
+    code_memory: CodeMemory,
+    trampoline_park: HashMap<VMSharedSignatureIndex, AtomicPtr<VMFunctionBody>>,
     /// The `FunctionBuilderContext`, shared between trampline function compilations.
     fn_builder_ctx: FunctionBuilderContext,
 }
@@ -71,13 +78,15 @@ impl Compiler {
     ) -> Self {
         Self {
             isa,
-            code_memory: CodeMemory::new(),
-            trampoline_park: HashMap::new(),
             signatures: SignatureRegistry::new(),
-            fn_builder_ctx: FunctionBuilderContext::new(),
             strategy,
             trap_registry: TrapRegistry::default(),
             cache_config,
+            inner: RwLock::new(CompilerInner {
+                code_memory: CodeMemory::new(),
+                trampoline_park: HashMap::new(),
+                fn_builder_ctx: FunctionBuilderContext::new(),
+            }),
         }
     }
 }
@@ -95,14 +104,14 @@ impl Compiler {
 
     /// Compile the given function bodies.
     pub(crate) fn compile<'data>(
-        &mut self,
+        &self,
         module: &Module,
         module_translation: &ModuleTranslationState,
         function_body_inputs: PrimaryMap<DefinedFuncIndex, FunctionBodyData<'data>>,
         debug_data: Option<DebugInfoData>,
     ) -> Result<
         (
-            PrimaryMap<DefinedFuncIndex, *mut [VMFunctionBody]>,
+            PrimaryMap<DefinedFuncIndex, *const [VMFunctionBody]>,
             PrimaryMap<DefinedFuncIndex, ir::JumpTableOffsets>,
             Relocations,
             Option<Vec<u8>>,
@@ -138,8 +147,9 @@ impl Compiler {
             }
             .map_err(SetupError::Compile)?;
 
-        let allocated_functions =
-            allocate_functions(&mut self.code_memory, &compilation).map_err(|message| {
+        let mut inner = self.inner.write().unwrap();
+        let allocated_functions = allocate_functions(&mut inner.code_memory, &compilation)
+            .map_err(|message| {
                 SetupError::Instantiate(InstantiationError::Resource(format!(
                     "failed to allocate memory for functions: {}",
                     message
@@ -201,48 +211,52 @@ impl Compiler {
 
     /// Create a trampoline for invoking a function.
     pub(crate) fn get_trampoline(
-        &mut self,
+        &self,
         signature: &ir::Signature,
         value_size: usize,
-    ) -> Result<*const VMFunctionBody, SetupError> {
+    ) -> Result<*mut VMFunctionBody, SetupError> {
         let index = self.signatures.register(signature);
-        if let Some(trampoline) = self.trampoline_park.get(&index) {
-            return Ok(*trampoline);
+        let inner = &mut *self.inner.write().unwrap();
+        if let Some(trampoline) = inner.trampoline_park.get(&index) {
+            return Ok(trampoline.load(Ordering::SeqCst));
         }
         let body = make_trampoline(
             &*self.isa,
-            &mut self.code_memory,
-            &mut self.fn_builder_ctx,
+            &mut inner.code_memory,
+            &mut inner.fn_builder_ctx,
             signature,
             value_size,
         )?;
-        self.trampoline_park.insert(index, body);
+        inner.trampoline_park.insert(index, AtomicPtr::new(body));
         return Ok(body);
     }
 
     /// Create and publish a trampoline for invoking a function.
     pub fn get_published_trampoline(
-        &mut self,
+        &self,
         signature: &ir::Signature,
         value_size: usize,
-    ) -> Result<*const VMFunctionBody, SetupError> {
+    ) -> Result<*mut VMFunctionBody, SetupError> {
         let result = self.get_trampoline(signature, value_size)?;
         self.publish_compiled_code();
         Ok(result)
     }
 
     /// Make memory containing compiled code executable.
-    pub(crate) fn publish_compiled_code(&mut self) {
-        self.code_memory.publish();
+    pub(crate) fn publish_compiled_code(&self) {
+        self.inner.write().unwrap().code_memory.publish();
     }
 
     pub(crate) fn profiler_module_load(
-        &mut self,
+        &self,
         profiler: &mut Box<dyn ProfilingAgent + Send>,
         module_name: &str,
         dbg_image: Option<&[u8]>,
     ) -> () {
-        self.code_memory
+        self.inner
+            .write()
+            .unwrap()
+            .code_memory
             .profiler_module_load(profiler, module_name, dbg_image);
     }
 
@@ -264,7 +278,7 @@ fn make_trampoline(
     fn_builder_ctx: &mut FunctionBuilderContext,
     signature: &ir::Signature,
     value_size: usize,
-) -> Result<*const VMFunctionBody, SetupError> {
+) -> Result<*mut VMFunctionBody, SetupError> {
     let pointer_type = isa.pointer_type();
     let mut wrapper_sig = ir::Signature::new(isa.frontend_config().default_call_conv);
 
@@ -373,26 +387,26 @@ fn make_trampoline(
             unwind_info,
         })
         .map_err(|message| SetupError::Instantiate(InstantiationError::Resource(message)))?
-        .as_ptr())
+        .as_mut_ptr())
 }
 
 fn allocate_functions(
     code_memory: &mut CodeMemory,
     compilation: &Compilation,
-) -> Result<PrimaryMap<DefinedFuncIndex, *mut [VMFunctionBody]>, String> {
+) -> Result<PrimaryMap<DefinedFuncIndex, *const [VMFunctionBody]>, String> {
     let fat_ptrs = code_memory.allocate_for_compilation(compilation)?;
 
     // Second, create a PrimaryMap from result vector of pointers.
     let mut result = PrimaryMap::with_capacity(compilation.len());
     for i in 0..fat_ptrs.len() {
-        let fat_ptr: *mut [VMFunctionBody] = fat_ptrs[i];
+        let fat_ptr: *const [VMFunctionBody] = fat_ptrs[i];
         result.push(fat_ptr);
     }
     Ok(result)
 }
 
 fn register_traps(
-    allocated_functions: &PrimaryMap<DefinedFuncIndex, *mut [VMFunctionBody]>,
+    allocated_functions: &PrimaryMap<DefinedFuncIndex, *const [VMFunctionBody]>,
     traps: &Traps,
     registry: &TrapRegistry,
 ) -> TrapRegistration {