Skip to content

Invoking UB if OkGuard and ErrGuard aren't dropped is unsound #113

@taylordotfish

Description

@taylordotfish

Currently, the documentation for OkGuard and ErrGuard states (emphasis mine):

When dropped, this guard ensures that the result’s determinant is properly set. Failing to drop this guard may result in undefined behaviour.

However, it is unsound to invoke undefined behavior if a destructor isn't run, because it's always possible to prevent a destructor from running in safe Rust using mem::forget or by creating a cyclic Rc.

In the case of match_mut and match_mut_ctx, I think this can be fixed by making the closures take a plain reference, and using the guard types only as an internal implementation detail of the parent function, where their destructors are guaranteed to run:

     pub fn match_mut<
         'a,
         U,
-        FnOk: FnOnce(OkGuard<'a, Ok, Err>) -> U,
-        FnErr: FnOnce(ErrGuard<'a, Ok, Err>) -> U,
+        FnOk: FnOnce(&mut Ok) -> U,
+        FnErr: FnOnce(&mut Err) -> U,
     >(
         &'a mut self,
         ok: FnOk,
         err: FnErr,
     ) -> U {
         let r;
         if Self::is_ok(self) {
             unsafe {
-                 r = ok(self.ok_mut_unchecked());
+                 r = ok(&mut *self.ok_mut_unchecked());
             }
         } else {
             unsafe {
-                r = err(self.err_mut_unchecked());
+                r = err(&mut *self.err_mut_unchecked());
             }
         }
         r
     }

In the case of ok_mut and err_mut, though, I'm not sure how to fix this. The only approaches I can think of are:

  • Implementing poisoning, where calling ok_mut poisons the Result, and dropping the OkGuard unpoisons it, and attempting to access a poisoned Result produces a well-defined panic instead of UB.
  • Marking the methods unsafe, and requiring the caller to ensure that if the guard goes out of scope without being dropped, the Result will not be accessed.

Neither of these approaches seem ideal.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions