Bug 1716518 - Upgrade ffi-support to v0.4.3. r=emilio

Differential Revision: https://phabricator.services.mozilla.com/D117787

Cargo.lock

@@ -1395,9 +1395,9 @@ dependencies = [
 
 [[package]]
 name = "ffi-support"
-version = "0.4.2"
+version = "0.4.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f85d4d1be103c0b2d86968f0b0690dc09ac0ba205b90adb0389b552869e5000e"
+checksum = "6f476e3c4fa6d9a3e0eeabd4f7555a382025ec05194675d72fa1db347e53ae6c"
 dependencies = [
  "lazy_static",
  "log",

third_party/rust/ffi-support/.cargo-checksum.json

@@ -1 +1 @@
-{"files":{"Cargo.toml":"59a3a656f31bd225f1864abff65b678bdc5c863e14e426f980d4d8b02f1ca747","LICENSE-APACHE":"a60eea817514531668d7e00765731449fe14d059d3249e0bc93b36de45f759f2","LICENSE-MIT":"63e747d86bdeb67638f26b4b75107f129c5f12de432ae83ccdb1ccbe28debf30","README.md":"ac7041bc517c1fc051fa9773527955c6debc12473f80b9f24bea7119c3fb35be","src/error.rs":"e4c87fe305f7fbb830348c0f5b181385c96aa9561dfa1536ef1ce09065e6ce83","src/ffistr.rs":"12a4f351c248e150da18b6ea3797eca65f63e8fa24c62828a2510b9c3a4b8ca5","src/handle_map.rs":"d6ac073e1559aad561e32a8c2b7b18257fd7f70c1cf4004a8f90ed6202a185e8","src/into_ffi.rs":"bde79bf6b2bbc3108654bf14ac4216c4c5f2a91962de6f814d0bac1bde243c1c","src/lib.rs":"fcdb5231573677f8d1fa7b37b36319a688dc459d8a56c5dd9144eace25b22c94","src/macros.rs":"479153198e1676fdca0be53cbd437a05cd807a2520bacfdb8849a742188f1359","src/string.rs":"966d2b41fae4e7a6083eb142a57e669e4bafd833f01c8b24fc67dff4fb4a5595"},"package":"f85d4d1be103c0b2d86968f0b0690dc09ac0ba205b90adb0389b552869e5000e"}
+{"files":{"CODE_OF_CONDUCT.md":"e85149c44f478f164f7d5f55f6e66c9b5ae236d4a11107d5e2a93fe71dd874b9","Cargo.toml":"26fb28d702092a5e4875d272b3ad6e607c3865457a4c929b0239fe3fa1f7ef17","LICENSE-APACHE":"a60eea817514531668d7e00765731449fe14d059d3249e0bc93b36de45f759f2","LICENSE-MIT":"63e747d86bdeb67638f26b4b75107f129c5f12de432ae83ccdb1ccbe28debf30","README.md":"8948f6ed32931fcf5aafef112029dcd5bccf1452403e067bcfd69ba5fcbb5782","src/error.rs":"9331b440e061df76d456026e41fd29124b454fca9198005e725885e17466ba3d","src/ffistr.rs":"12a4f351c248e150da18b6ea3797eca65f63e8fa24c62828a2510b9c3a4b8ca5","src/handle_map.rs":"f2f66b7c2463d3fa2dc087ac15d21b43cbfde7ec9eec361b2950a7cd20d15b5c","src/into_ffi.rs":"2c79df8ecd775cd3ef9991dbd43f7be138f74696f639a77b93d3783d32c72ca6","src/lib.rs":"fcdb5231573677f8d1fa7b37b36319a688dc459d8a56c5dd9144eace25b22c94","src/macros.rs":"e1e17a68aa16b6a1ec271d91e070eecbe68da033a3c90d272d9e984e3c78dbd1","src/string.rs":"966d2b41fae4e7a6083eb142a57e669e4bafd833f01c8b24fc67dff4fb4a5595","tests/test.rs":"1b3a86eaa21706e3c8041b3e27b33c30baf14b9ccddfb932db985c4e2f702460"},"package":"6f476e3c4fa6d9a3e0eeabd4f7555a382025ec05194675d72fa1db347e53ae6c"}

third_party/rust/ffi-support/CODE_OF_CONDUCT.md

@@ -0,0 +1,25 @@
+# Community Participation Guidelines
+
+This repository is governed by Mozilla's code of conduct and etiquette guidelines.
+For more details, please read the
+[Mozilla Community Participation Guidelines](https://www.mozilla.org/about/governance/policies/participation/).
+
+## How to Report
+For more information on how to report violations of the Community Participation Guidelines, please read our '[How to Report](https://www.mozilla.org/about/governance/policies/participation/reporting/)' page.
+
+## Project Specific Etiquette
+
+### Our Responsibilities
+
+Project maintainers are responsible for clarifying the standards of acceptable
+behavior and are expected to take appropriate and fair corrective action in
+response to any instances of unacceptable behavior.
+
+Project maintainers have the right and responsibility to remove, edit, or
+reject comments, commits, code, wiki edits, issues, and other contributions
+that are not aligned to this Code of Conduct, or to ban temporarily or
+permanently any contributor for other behaviors that they deem inappropriate,
+threatening, offensive, or harmful.
+
+Project maintainers who do not follow or enforce Mozilla's Participation Guidelines in good
+faith may face temporary or permanent repercussions.

third_party/rust/ffi-support/Cargo.toml

@@ -13,7 +13,7 @@
 [package]
 edition = "2018"
 name = "ffi-support"
-version = "0.4.2"
+version = "0.4.3"
 authors = ["Thom Chiovoloni <tchiovoloni@mozilla.com>"]
 description = "A crate to help expose Rust functions over the FFI."
 readme = "README.md"
@@ -22,7 +22,7 @@ categories = ["development-tools::ffi"]
 license = "Apache-2.0 / MIT"
 repository = "https://github.com/mozilla/application-services"
 [dependencies.backtrace]
-version = "0.3.48"
+version = "0.3"
 optional = true
 
 [dependencies.lazy_static]
@@ -30,10 +30,20 @@ version = "1.4"
 
 [dependencies.log]
 version = "0.4"
+[dev-dependencies.env_logger]
+version = "0.7"
+default-features = false
+
+[dev-dependencies.log]
+version = "0.4"
+
+[dev-dependencies.rand]
+version = "0.7"
+
+[dev-dependencies.rayon]
+version = "1.3"
 
 [features]
 default = []
 log_backtraces = ["log_panics", "backtrace"]
 log_panics = []
-[badges.travis-ci]
-repository = "mozilla/application-services"

third_party/rust/ffi-support/README.md

@@ -18,7 +18,7 @@ Additionally, it's documentation describes a number of the problems we've hit do
 Add the following to your Cargo.toml
 
 ```toml
-ffi-support = "0.1.1"
+ffi-support = "0.4.3"
 ```
 
 For further examples, the examples in the docs is the best starting point, followed by the usage code in the [mozilla/application-services](https://github.com/mozilla/application-services) repo (for example [here](https://github.com/mozilla/application-services/blob/main/components/places/ffi/src/lib.rs) or [here](https://github.com/mozilla/application-services/blob/main/components/places/src/ffi.rs)).

third_party/rust/ffi-support/src/error.rs

@@ -208,7 +208,7 @@ impl ExternError {
         self.message as *const _
     }
 
-    /// Get the `message` property as an [`FfiStr`]
+    /// Get the `message` property as an [`FfiStr`][crate::FfiStr]
     #[inline]
     pub fn get_message(&self) -> crate::FfiStr<'_> {
         // Safe because the lifetime is the same as our lifetime.

third_party/rust/ffi-support/src/handle_map.rs

@@ -900,7 +900,7 @@ impl<T> ConcurrentHandleMap<T> {
         self.get(Handle::from_u64(u)?, callback)
     }
 
-    /// Convenient wrapper for `get_mut` which takes a `u64` that it will
+    /// Convenient wrapper for [`Self::get_mut`] which takes a `u64` that it will
     /// convert to a handle.
     ///
     /// The main benefit (besides convenience) of this over the version
@@ -925,7 +925,9 @@ impl<T> ConcurrentHandleMap<T> {
         self.get_mut(Handle::from_u64(u)?, callback)
     }
 
-    /// Helper that performs both a [`call_with_result`] and [`get`](ConcurrentHandleMap::get_mut).
+    /// Helper that performs both a
+    /// [`call_with_result`][crate::call_with_result] and
+    /// [`get`](ConcurrentHandleMap::get_mut).
     pub fn call_with_result_mut<R, E, F>(
         &self,
         out_error: &mut ExternError,
@@ -949,7 +951,9 @@ impl<T> ConcurrentHandleMap<T> {
         })
     }
 
-    /// Helper that performs both a [`call_with_result`] and [`get`](ConcurrentHandleMap::get).
+    /// Helper that performs both a
+    /// [`call_with_result`][crate::call_with_result] and
+    /// [`get`](ConcurrentHandleMap::get).
     pub fn call_with_result<R, E, F>(
         &self,
         out_error: &mut ExternError,
@@ -964,7 +968,9 @@ impl<T> ConcurrentHandleMap<T> {
         self.call_with_result_mut(out_error, h, |r| callback(r))
     }
 
-    /// Helper that performs both a [`call_with_output`] and [`get`](ConcurrentHandleMap::get).
+    /// Helper that performs both a
+    /// [`call_with_output`][crate::call_with_output] and
+    /// [`get`](ConcurrentHandleMap::get).
     pub fn call_with_output<R, F>(
         &self,
         out_error: &mut ExternError,
@@ -980,7 +986,9 @@ impl<T> ConcurrentHandleMap<T> {
         })
     }
 
-    /// Helper that performs both a [`call_with_output`] and [`get_mut`](ConcurrentHandleMap::get).
+    /// Helper that performs both a
+    /// [`call_with_output`][crate::call_with_output] and
+    /// [`get_mut`](ConcurrentHandleMap::get).
     pub fn call_with_output_mut<R, F>(
         &self,
         out_error: &mut ExternError,
@@ -997,8 +1005,8 @@ impl<T> ConcurrentHandleMap<T> {
     }
 
     /// Use `constructor` to create and insert a `T`, while inside a
-    /// [`call_with_result`] call (to handle panics and map errors onto an
-    /// `ExternError`).
+    /// [`call_with_result`][crate::call_with_result] call (to handle panics and
+    /// map errors onto an [`ExternError`][crate::ExternError]).
     pub fn insert_with_result<E, F>(&self, out_error: &mut ExternError, constructor: F) -> u64
     where
         F: std::panic::UnwindSafe + FnOnce() -> Result<T, E>,
@@ -1018,8 +1026,9 @@ impl<T> ConcurrentHandleMap<T> {
     /// [`insert_with_result`](ConcurrentHandleMap::insert_with_result) for the
     /// case where the constructor cannot produce an error.
     ///
-    /// The name is somewhat dubious, since there's no `output`, but it's intended to make it
-    /// clear that it contains a [`call_with_output`] internally.
+    /// The name is somewhat dubious, since there's no `output`, but it's
+    /// intended to make it clear that it contains a
+    /// [`call_with_output`][crate::call_with_output] internally.
     pub fn insert_with_output<F>(&self, out_error: &mut ExternError, constructor: F) -> u64
     where
         F: std::panic::UnwindSafe + FnOnce() -> T,

third_party/rust/ffi-support/src/into_ffi.rs

@@ -15,7 +15,7 @@
  * limitations under the Licenses. */
 
 use crate::string::*;
-use std::os::raw::c_char;
+use std::os::raw::{c_char, c_void};
 use std::ptr;
 
 /// This trait is used to return types over the FFI. It essentially is a mapping between a type and
@@ -277,4 +277,11 @@ macro_rules! impl_into_ffi_for_pointer {
     )+}
 }
 
-impl_into_ffi_for_pointer![*mut i8, *const i8, *mut u8, *const u8];
+impl_into_ffi_for_pointer![
+    *mut i8,
+    *const i8,
+    *mut u8,
+    *const u8,
+    *mut c_void,
+    *const c_void
+];

third_party/rust/ffi-support/src/macros.rs

@@ -14,17 +14,19 @@
  * See the Licenses for the specific language governing permissions and
  * limitations under the Licenses. */
 
-/// Implements [`IntoFfi`] for the provided types (more than one may be passed in) by allocating
-/// `$T` on the heap as an opaque pointer.
+/// Implements [`IntoFfi`][crate::IntoFfi] for the provided types (more than one
+/// may be passed in) by allocating `$T` on the heap as an opaque pointer.
 ///
-/// This is typically going to be used from the "Rust component", and not the "FFI component" (see
-/// the top level crate documentation for more information), however you will still need to
-/// implement a destructor in the FFI component using [`define_box_destructor!`].
+/// This is typically going to be used from the "Rust component", and not the
+/// "FFI component" (see the top level crate documentation for more
+/// information), however you will still need to implement a destructor in the
+/// FFI component using [`define_box_destructor!`][crate::define_box_destructor].
 ///
-/// In general, is only safe to do for `send` types (even this is dodgy, but it's often necessary
-/// to keep the locking on the other side of the FFI, so Sync is too harsh), so we enforce this in
-/// this macro. (You're still free to implement this manually, if this restriction is too harsh
-/// for your use case and you're certain you know what you're doing).
+/// In general, is only safe to do for `send` types (even this is dodgy, but
+/// it's often necessary to keep the locking on the other side of the FFI, so
+/// Sync is too harsh), so we enforce this in this macro. (You're still free to
+/// implement this manually, if this restriction is too harsh for your use case
+/// and you're certain you know what you're doing).
 #[macro_export]
 macro_rules! implement_into_ffi_by_pointer {
     ($($T:ty),* $(,)*) => {$(
@@ -44,8 +46,8 @@ macro_rules! implement_into_ffi_by_pointer {
     )*}
 }
 
-/// Implements [`IntoFfi`] for the provided types (more than one may be passed
-/// in) by converting to the type to a JSON string.
+/// Implements [`IntoFfi`][crate::IntoFfi] for the provided types (more than one
+/// may be passed in) by converting to the type to a JSON string.
 ///
 /// Additionally, most of the time we recomment using this crate's protobuf
 /// support, instead of JSON.
@@ -61,8 +63,8 @@ macro_rules! implement_into_ffi_by_pointer {
 ///
 /// ## Panics
 ///
-/// The [`IntoFfi`] implementation this macro generates may panic in the
-/// following cases:
+/// The [`IntoFfi`][crate::IntoFfi] implementation this macro generates may
+/// panic in the following cases:
 ///
 /// - You've passed a type that contains a Map that has non-string keys (which
 ///   can't be represented in JSON).
@@ -97,9 +99,10 @@ macro_rules! implement_into_ffi_by_json {
     )*}
 }
 
-/// Implements [`IntoFfi`] for the provided types (more than one may be passed in) implementing
-/// `prost::Message` (protobuf auto-generated type) by converting to the type to a [`ByteBuffer`].
-/// This [`ByteBuffer`] should later be passed by value.
+/// Implements [`IntoFfi`][crate::IntoFfi] for the provided types (more than one
+/// may be passed in) implementing `prost::Message` (protobuf auto-generated
+/// type) by converting to the type to a [`ByteBuffer`][crate::ByteBuffer]. This
+/// [`ByteBuffer`][crate::ByteBuffer] should later be passed by value.
 ///
 /// Note: for this to works, the crate it's called in must depend on `prost`.
 ///
@@ -127,14 +130,15 @@ macro_rules! implement_into_ffi_by_protobuf {
     )*}
 }
 
-/// Implement IntoFfi for a type by converting through another type.
+/// Implement [`InfoFfi`][crate::IntoFfi] for a type by converting through
+/// another type.
 ///
 /// The argument `$MidTy` argument must implement `From<$SrcTy>` and
-/// [`InfoFfi`].
+/// [`InfoFfi`][crate::IntoFfi].
 ///
 /// This is provided (even though it's trivial) because it is always safe (well,
-/// so long as `$MidTy`'s [`IntoFfi`] implementation is correct), but would
-/// otherwise require use of `unsafe` to implement.
+/// so long as `$MidTy`'s [`IntoFfi`][crate::IntoFfi] implementation is
+/// correct), but would otherwise require use of `unsafe` to implement.
 #[macro_export]
 macro_rules! implement_into_ffi_by_delegation {
     ($SrcTy:ty, $MidTy:ty) => {
@@ -166,7 +170,7 @@ macro_rules! implement_into_ffi_by_delegation {
 /// provide it as a macro.
 ///
 /// It simply expands to a `#[no_mangle] pub unsafe extern "C" fn` which wraps this crate's
-/// [`destroy_c_string`] function.
+/// [`destroy_c_string`][crate::destroy_c_string] function.
 ///
 /// ## Caveats
 ///
@@ -263,7 +267,7 @@ macro_rules! define_box_destructor {
     };
 }
 
-/// Define a (public) destructor for the ByteBuffer type.
+/// Define a (public) destructor for the [`ByteBuffer`][crate::ByteBuffer] type.
 ///
 /// ## Caveats
 ///
@@ -296,7 +300,7 @@ macro_rules! define_bytebuffer_destructor {
 }
 
 /// Define a (public) destructor for a type that lives inside a lazy_static
-/// [`ConcurrentHandleMap`].
+/// [`ConcurrentHandleMap`][crate::ConcurrentHandleMap].
 ///
 /// Note that this is actually totally safe, unlike the other
 /// `define_blah_destructor` macros.

third_party/rust/ffi-support/tests/test.rs

@@ -0,0 +1,102 @@
+/* Copyright 2018-2019 Mozilla Foundation
+ *
+ * Licensed under the Apache License (Version 2.0), or the MIT license,
+ * (the "Licenses") at your option. You may not use this file except in
+ * compliance with one of the Licenses. You may obtain copies of the
+ * Licenses at:
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *    http://opensource.org/licenses/MIT
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the Licenses is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the Licenses for the specific language governing permissions and
+ * limitations under the Licenses.
+ */
+
+//! This test is a stress test meant to trigger some bugs seen prior to the use
+//! of handlemaps. See /docs/design/test-faster.md for why it's split -- TLDR:
+//! it uses rayon and is hard to rewrite with normal threads.
+
+use ffi_support::{ConcurrentHandleMap, ExternError};
+use std::sync::atomic::{AtomicUsize, Ordering};
+use std::sync::Arc;
+
+fn with_error<F: FnOnce(&mut ExternError) -> T, T>(callback: F) -> T {
+    let mut e = ExternError::success();
+    let result = callback(&mut e);
+    if let Some(m) = unsafe { e.get_and_consume_message() } {
+        panic!("unexpected error: {}", m);
+    }
+    result
+}
+
+struct DropChecking {
+    counter: Arc<AtomicUsize>,
+    id: usize,
+}
+impl Drop for DropChecking {
+    fn drop(&mut self) {
+        let val = self.counter.fetch_add(1, Ordering::SeqCst);
+        log::debug!("Dropped {} :: {}", self.id, val);
+    }
+}
+#[test]
+fn test_concurrent_drop() {
+    use rand::prelude::*;
+    use rayon::prelude::*;
+    let _ = env_logger::try_init();
+    let drop_counter = Arc::new(AtomicUsize::new(0));
+    let id = Arc::new(AtomicUsize::new(1));
+    let map = ConcurrentHandleMap::new();
+    let count = 1000;
+    let mut handles = (0..count)
+        .into_par_iter()
+        .map(|_| {
+            let id = id.fetch_add(1, Ordering::SeqCst);
+            let handle = with_error(|e| {
+                map.insert_with_output(e, || {
+                    log::debug!("Created {}", id);
+                    DropChecking {
+                        counter: drop_counter.clone(),
+                        id,
+                    }
+                })
+            });
+            (id, handle)
+        })
+        .collect::<Vec<_>>();
+
+    handles.shuffle(&mut thread_rng());
+
+    assert_eq!(drop_counter.load(Ordering::SeqCst), 0);
+    handles.par_iter().for_each(|(id, h)| {
+        with_error(|e| {
+            map.call_with_output(e, *h, |val| {
+                assert_eq!(val.id, *id);
+            })
+        });
+    });
+
+    assert_eq!(drop_counter.load(Ordering::SeqCst), 0);
+
+    handles.par_iter().for_each(|(id, h)| {
+        with_error(|e| {
+            map.call_with_output(e, *h, |val| {
+                assert_eq!(val.id, *id);
+            })
+        });
+    });
+
+    handles.par_iter().for_each(|(id, h)| {
+        let item = map
+            .remove_u64(*h)
+            .expect("remove to succeed")
+            .expect("item to exist");
+        assert_eq!(item.id, *id);
+        let h = map.insert(item).into_u64();
+        map.delete_u64(h).expect("delete to succeed");
+    });
+    assert_eq!(drop_counter.load(Ordering::SeqCst), count);
+}