fix bug where join_timeout would kill the subprocess on first timeout

since join_timeout does no longer consume the handle, killing it on
timeout is not appropriate. instead move that to a drop impl

also add test for new join_timeout behavior

Author: untitaker

src/pool.rs

@@ -63,7 +63,6 @@ impl<T: Serialize + DeserializeOwned> PooledHandle<T> {
             Ok(Ok(rv)) => Ok(rv),
             Ok(Err(err)) => Err(err),
             Err(mpsc::RecvTimeoutError::Timeout) => {
-                self.kill().ok();
                 Err(SpawnError::new_timeout())
             }
             Err(mpsc::RecvTimeoutError::Disconnected) => Err(SpawnError::new_remote_close()),

src/proc.rs

@@ -515,11 +515,15 @@ impl<T: Serialize + DeserializeOwned> JoinHandle<T> {
     /// Wait for the child process to return a result.
     ///
     /// If the join handle was created from a pool the join is virtualized.
-    pub fn join(self) -> Result<T, SpawnError> {
-        match self.inner {
-            Ok(JoinHandleInner::Process(mut handle)) => handle.join(),
-            Ok(JoinHandleInner::Pooled(mut handle)) => handle.join(),
-            Err(err) => Err(err),
+    pub fn join(mut self) -> Result<T, SpawnError> {
+        match &mut self.inner {
+            Ok(JoinHandleInner::Process(ref mut handle)) => handle.join(),
+            Ok(JoinHandleInner::Pooled(ref mut handle)) => handle.join(),
+            Err(err) => {
+                let mut rv_err = SpawnError::new_consumed();
+                mem::swap(&mut rv_err, err);
+                Err(rv_err)
+            }
         }
     }
 
@@ -551,6 +555,12 @@ impl<T: Serialize + DeserializeOwned> JoinHandle<T> {
     }
 }
 
+impl<T> Drop for JoinHandle<T> {
+    fn drop(&mut self) {
+        self.kill().ok();
+    }
+}
+
 /// Spawn a new process to run a function with some payload.
 ///
 /// ```rust,no_run

tests/test_pool.rs

@@ -77,3 +77,23 @@ fn test_timeout() {
     let val = handle.join_timeout(Duration::from_secs(2)).unwrap();
     assert_eq!(val, 42);
 }
+
+
+#[test]
+fn test_timeout_twice() {
+    let pool = Pool::new(2).unwrap();
+
+    let mut handle = pool.spawn((), |()| {
+        thread::sleep(Duration::from_secs(5));
+        42
+    });
+
+    let err = handle.join_timeout(Duration::from_millis(100)).unwrap_err();
+    assert!(err.is_timeout());
+
+    let err = handle.join_timeout(Duration::from_millis(100)).unwrap_err();
+    assert!(err.is_timeout());
+
+    let val = handle.join_timeout(Duration::from_secs(6)).unwrap();
+    assert_eq!(val, 42);
+}