Remove global engine info lock

gpg-rs · Dec 31, 2023 · 4b016f7 · 4b016f7
1 parent 1d366a1
commit 4b016f7
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 61 deletions.
diff --git a/src/context.rs b/src/context.rs
@@ -17,7 +17,7 @@ use libc;
 #[allow(deprecated)]
 use crate::{
     callbacks, edit,
-    engine::EngineInfo,
+    engine::{EngineInfo, EngineInfos},
     notation::SignatureNotations,
     results,
     utils::{convert_err, CStrArgument, SmallVec},
@@ -42,7 +42,7 @@ impl Drop for Context {
 impl Context {
     impl_wrapper!(ffi::gpgme_ctx_t);
 
-    fn new() -> Result<Self> {
+    pub(crate) fn new() -> Result<Self> {
         crate::init();
         unsafe {
             let mut ctx = ptr::null_mut();
@@ -163,6 +163,13 @@ impl Context {
         unsafe { EngineInfo::from_raw(ffi::gpgme_ctx_get_engine_info(self.as_raw())) }
     }
 
+    /// Upstream documentation:
+    /// [`gpgme_ctx_get_engine_info`](https://www.gnupg.org/documentation/manuals/gpgme/Crypto-Engine.html#index-gpgme_005fctx_005fget_005fengine_005finfo)
+    #[inline]
+    pub fn engines(&self) -> EngineInfos<'_> {
+        unsafe { EngineInfos::from_list(ffi::gpgme_ctx_get_engine_info(self.as_raw())) }
+    }
+
     #[inline]
     pub fn set_engine_path(&mut self, path: impl CStrArgument) -> Result<()> {
         let path = path.into_cstr();
@@ -729,11 +736,11 @@ impl Context {
         let expires = expires.as_secs().value_into().unwrap_or_saturate();
         self::with_joined_cstr(subkeys, |subkeys, _| unsafe {
             convert_err(ffi::gpgme_op_setexpire(
-                    self.as_raw(),
-                    key.as_raw(),
-                    expires,
-                    subkeys.map_or(ptr::null(), |subkeys| subkeys.as_ptr()),
-                    0,
+                self.as_raw(),
+                key.as_raw(),
+                expires,
+                subkeys.map_or(ptr::null(), |subkeys| subkeys.as_ptr()),
+                0,
             ))
         })
     }

diff --git a/src/engine.rs b/src/engine.rs
@@ -9,7 +9,7 @@ use std::{
 
 use ffi;
 
-use crate::{utils::convert_err, NonNull, Protocol, Result};
+use crate::{utils::convert_err, Context, NonNull, Protocol, Result};
 
 /// Upstream documentation:
 /// [`gpgme_engine_info_t`](https://www.gnupg.org/documentation/manuals/gpgme/Engine-Information.html#index-gpgme_005fengine_005finfo_005ft)
@@ -116,16 +116,15 @@ impl_list_iterator!(pub struct EngineInfos(EngineInfo: ffi::gpgme_engine_info_t)
 
 /// A RAII guard type that ensures the global engine information list is not modified
 /// while it is being iterated.
-pub struct EngineInfoGuard(RwLockReadGuard<'static, ()>);
+pub struct EngineInfoGuard {
+    snapshot: Context,
+}
 
 impl EngineInfoGuard {
-    pub fn new(lock: &'static RwLock<()>) -> Result<EngineInfoGuard> {
-        let lock = lock.read().expect("engine info lock was poisoned");
-        unsafe {
-            let mut info = ptr::null_mut();
-            convert_err(ffi::gpgme_get_engine_info(&mut info))?;
-        }
-        Ok(EngineInfoGuard(lock))
+    pub(crate) fn new() -> Result<Self> {
+        Ok(Self {
+            snapshot: Context::new()?,
+        })
     }
 
     #[inline]
@@ -145,11 +144,7 @@ impl<'a> IntoIterator for &'a EngineInfoGuard {
 
     #[inline]
     fn into_iter(self) -> Self::IntoIter {
-        unsafe {
-            let mut first = ptr::null_mut();
-            assert_eq!(ffi::gpgme_get_engine_info(&mut first), 0);
-            EngineInfos::from_list(first)
-        }
+        self.snapshot.engines()
     }
 }
 

diff --git a/src/lib.rs b/src/lib.rs
@@ -2,7 +2,7 @@ use std::{
     ffi::CStr,
     fmt, ptr,
     str::Utf8Error,
-    sync::{Mutex, OnceLock, RwLock},
+    sync::{Mutex, OnceLock},
 };
 
 use self::{
@@ -180,7 +180,7 @@ cfg_if::cfg_if! {
 /// ```
 #[inline]
 pub fn init() -> Gpgme {
-    static TOKEN: OnceLock<(&str, RwLock<()>)> = OnceLock::new();
+    static TOKEN: OnceLock<&str> = OnceLock::new();
     let token = TOKEN.get_or_init(|| unsafe {
         let offset = memoffset::offset_of!(ffi::_gpgme_signature, validity);
 
@@ -189,24 +189,17 @@ pub fn init() -> Gpgme {
             !result.is_null(),
             "the library linked is not the correct version"
         );
-        (
-            CStr::from_ptr(result)
-                .to_str()
-                .expect("gpgme version string is not valid utf-8"),
-            RwLock::default(),
-        )
+        CStr::from_ptr(result)
+            .to_str()
+            .expect("gpgme version string is not valid utf-8")
     });
-    Gpgme {
-        version: token.0,
-        engine_lock: &token.1,
-    }
+    Gpgme { version: token }
 }
 
 /// A type for managing the library's configuration.
 #[derive(Debug, Clone)]
 pub struct Gpgme {
     version: &'static str,
-    engine_lock: &'static RwLock<()>,
 }
 
 impl Gpgme {
@@ -289,31 +282,19 @@ impl Gpgme {
     /// [`gpgme_get_engine_info`](https://www.gnupg.org/documentation/manuals/gpgme/Engine-Information.html#index-gpgme_005fget_005fengine_005finfo)
     #[inline]
     pub fn engine_info(&self) -> Result<EngineInfoGuard> {
-        EngineInfoGuard::new(self.engine_lock)
-    }
-
-    // Requires the engine_lock to be held by the current thread when called
-    unsafe fn get_engine_info(&self, proto: Protocol) -> ffi::gpgme_engine_info_t {
-        let mut info = ptr::null_mut();
-        assert_eq!(ffi::gpgme_get_engine_info(&mut info), 0);
-        while !info.is_null() && ((*info).protocol != proto.raw()) {
-            info = (*info).next;
-        }
-        info
+        EngineInfoGuard::new()
     }
 
     #[inline]
     pub fn set_engine_path(&self, proto: Protocol, path: impl CStrArgument) -> Result<()> {
         let path = path.into_cstr();
-        let _lock = self
-            .engine_lock
-            .write()
-            .expect("engine info lock was poisoned");
         unsafe {
             let home_dir = self
-                .get_engine_info(proto)
+                .engine_info()?
+                .get(proto)
                 .as_ref()
-                .map_or(ptr::null(), |x| x.home_dir);
+                .and_then(|x| x.home_dir_raw())
+                .map_or(ptr::null(), |x| x.as_ptr());
             convert_err(ffi::gpgme_set_engine_info(
                 proto.raw(),
                 path.as_ref().as_ptr(),
@@ -326,15 +307,13 @@ impl Gpgme {
     #[inline]
     pub fn set_engine_home_dir(&self, proto: Protocol, home_dir: impl CStrArgument) -> Result<()> {
         let home_dir = home_dir.into_cstr();
-        let _lock = self
-            .engine_lock
-            .write()
-            .expect("engine info lock was poisoned");
         unsafe {
             let path = self
-                .get_engine_info(proto)
+                .engine_info()?
+                .get(proto)
                 .as_ref()
-                .map_or(ptr::null(), |x| x.file_name);
+                .and_then(|x| x.path_raw())
+                .map_or(ptr::null(), |x| x.as_ptr());
             convert_err(ffi::gpgme_set_engine_info(
                 proto.raw(),
                 path,
@@ -360,10 +339,6 @@ impl Gpgme {
             .as_ref()
             .map_or(ptr::null(), |s| s.as_ref().as_ptr());
         unsafe {
-            let _lock = self
-                .engine_lock
-                .write()
-                .expect("engine info lock was poisoned");
             convert_err(ffi::gpgme_set_engine_info(proto.raw(), path, home_dir))?;
         }
         Ok(())