Properly mark everything as (very) unsafe

Author: CouleeApps

Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "fork-map"
-version = "0.1.1"
+version = "0.1.2"
 edition = "2021"
 license = "MIT"
 description = "A crate for running operations in a child process spawned by `fork()`"
@@ -16,6 +16,6 @@ readme = "README.md"
 [dependencies]
 anyhow = "1.0"
 libc = "0.2"
-serde = { version = "1.0", features = ["derive"] }
+serde = "1.0"
 serde-error = "0.1.2"
 serde_json = "1.0"

Cargo.toml

@@ -10,15 +10,17 @@ use fork_map::fork_map;
 
 pub fn do_with_fork(value: u64) -> u64 {
     // Spawn a child process with a copy-on-write copy of memory
-    fork_map(|| {
-        // Do some obnoxious operation with `value`
-        // * Maybe it leaks memory
-        // * Maybe it uses static resources unsafely and 
-        //   prevents multi-threaded operation
-        // * Maybe you couldn't figure out how to
-        //   send your data to a thread
-        Ok(value * 10)
-    }).unwrap()
+    unsafe {
+        fork_map(|| {
+            // Do some obnoxious operation with `value`
+            // * Maybe it leaks memory
+            // * Maybe it uses static resources unsafely and 
+            //   prevents multi-threaded operation
+            // * Maybe you couldn't figure out how to
+            //   send your data to a thread
+            Ok(value * 10)
+        }).unwrap()
+    }
     // Execution continues after the child process has exited
 }
 ```
@@ -43,10 +45,12 @@ pub fn main() {
     let results = my_big_list.into_par_iter().map(|item| {
         // Have each worker spawn a child process for the
         // operations we don't want polluting the parent's memory
-        fork_map(|| {
-            // Do your ugly operations here
-            Ok(item * 1234)
-        }).expect("fork_map succeeded")
+        unsafe {
+            fork_map(|| {
+                // Do your ugly operations here
+                Ok(item * 1234)
+            }).expect("fork_map succeeded")
+        }
     }).collect::<Vec<_>>();
 
     // Use results here
@@ -69,17 +73,23 @@ pub fn main() {
         .chunks(512)
         .map(|items| {
             // Now each child process does 512 items at once
-            fork_map(|| {
-                let mut results = vec![];
-                // Maybe this operation is only mildly heinous
-                // and we can do 512 of them before the child
-                // process needs to be restarted.
-                for item in items {
-                    results.push(item * 1234);
-                }
-                Ok(results)
-            }).expect("fork_map succeeded")
+            unsafe {
+                fork_map(|| {
+                    let mut results = vec![];
+                    // Maybe this operation is only mildly heinous
+                    // and we can do 512 of them before the child
+                    // process needs to be restarted.
+                    for item in items {
+                        results.push(item * 1234);
+                    }
+                    Ok(results)
+                }).expect("fork_map succeeded")
+            }
         })
         .collect::<Vec<_>>();
 }
 ```
+
+## Safety
+
+Due to the nature of `fork()`, this function is very unsound and likely violates most of Rust's guarantees about lifetimes, considering all of your memory gets duplicated into a second process, even though it calls `exit(0)` after your closure is executed. Any threads other than the one calling `fork_map` will not be present in the new process, so threaded lifetime guarantees are also violated. Don't even think about using async executors with this.

README.md

@@ -12,16 +12,18 @@ use serde::{Deserialize, Serialize};
 /// // Result type needs to implement serde::Serialize and serde::Deserialize
 /// pub fn do_with_fork(value: u64) -> u64 {
 ///     // Spawn a child process with a copy-on-write copy of memory
-///     fork_map(|| {
-///         // Do some obnoxious operation with `value`
-///         // * Maybe it leaks memory
-///         // * Maybe it uses static resources unsafely and
-///         //   prevents multi-threaded operation
-///         // * Maybe you couldn't figure out how to
-///         //   send your data to a thread
-///         Ok(value * 10)
-///     }).unwrap()
-///     // Execution continues after the child process has exited
+///     unsafe {
+///         fork_map(|| {
+///             // Do some obnoxious operation with `value`
+///             // * Maybe it leaks memory
+///             // * Maybe it uses static resources unsafely and
+///             //   prevents multi-threaded operation
+///             // * Maybe you couldn't figure out how to
+///             //   send your data to a thread
+///             Ok(value * 10)
+///         }).unwrap()
+///         // Execution continues after the child process has exited
+///     }
 /// }
 /// ```
 ///
@@ -40,51 +42,57 @@ use serde::{Deserialize, Serialize};
 ///     let results = my_big_list.into_par_iter().map(|item| {
 ///         // Have each worker spawn a child process for the
 ///         // operations we don't want polluting the parent's memory
-///         fork_map(|| {
-///             // Do your ugly operations here
-///             Ok(item * 1234)
-///         }).expect("fork_map succeeded")
+///         unsafe {
+///             fork_map(|| {
+///                 // Do your ugly operations here
+///                 Ok(item * 1234)
+///             }).expect("fork_map succeeded")
+///         }
 ///     }).collect::<Vec<_>>();
 ///
 ///     // Use results here
 /// }
 /// ```
-pub fn fork_map<F, R>(func: F) -> anyhow::Result<R>
+///
+/// # Safety
+///
+/// Due to the nature of `fork()`, this function is very unsound and likely violates most of Rust's
+/// guarantees about lifetimes, considering all of your memory gets duplicated into a second
+/// process, even though it calls `exit(0)` after your closure is executed. Any threads other than
+/// the one calling `fork_map` will not be present in the new process, so threaded lifetime
+/// guarantees are also violated. Don't even think about using async executors with this.
+pub unsafe fn fork_map<F, R>(func: F) -> anyhow::Result<R>
     where
         F: Fn() -> anyhow::Result<R>,
         R: Serialize + for<'a> Deserialize<'a>,
 {
-    // SAFETY: Probably not LOL, didn't crash on my box, use at your own risk, etc.
-
     // Pipe for sending the result from child to parent
     let mut pipe: [libc::c_int; 2] = [0; 2];
-    unsafe {
-        libc::pipe(pipe.as_mut_ptr());
-    }
+    libc::pipe(pipe.as_mut_ptr());
 
     // Here we go
-    let pid = unsafe { libc::fork() };
+    let pid = libc::fork();
     if pid == 0 {
         // Child
-        unsafe { libc::close(pipe[0]) };
+        libc::close(pipe[0]);
         let result = func().map_err(|e| serde_error::Error::new(&*e));
         let ser = serde_json::to_string(&result).unwrap_or("".to_string());
-        unsafe { libc::write(pipe[1], ser.as_ptr() as *const libc::c_void, ser.len()) };
-        unsafe { libc::close(pipe[1]) };
-        unsafe { libc::exit(0) };
+        libc::write(pipe[1], ser.as_ptr() as *const libc::c_void, ser.len());
+        libc::close(pipe[1]);
+        libc::exit(0);
     }
 
     // Parent
-    unsafe { libc::close(pipe[1]) };
+    libc::close(pipe[1]);
 
     // Read result from pipe
     let mut des = vec![];
     let des = loop {
         const BUF_SIZE: usize = 0x1000;
         let mut buf: [u8; BUF_SIZE] = [0; BUF_SIZE];
-        let count = unsafe { libc::read(pipe[0], buf.as_mut_ptr() as *mut libc::c_void, BUF_SIZE) };
+        let count = libc::read(pipe[0], buf.as_mut_ptr() as *mut libc::c_void, BUF_SIZE);
         if count < 0 {
-            break Err(anyhow!("io error: {}", unsafe { *libc::__error() }));
+            break Err(anyhow!("io error: {}", *libc::__error()));
         }
         des.extend_from_slice(&buf[0..(count as usize)]);
         // EOF signalled by less than the max bytes
@@ -94,7 +102,7 @@ pub fn fork_map<F, R>(func: F) -> anyhow::Result<R>
     };
 
     let mut status = 0;
-    unsafe { libc::waitpid(pid, &mut status, 0) };
+    libc::waitpid(pid, &mut status, 0);
 
     if status != 0 {
         return Err(anyhow!("Process returned non-zero status code {}", status));