Merge pull request #147 from Clikengo/fix-136-callback-segfault

fix #136: Segfault when pending future dropped

Author: Vincent Rouillé

foundationdb/src/future.rs

@@ -26,15 +26,15 @@ use error::{self, Error, Result};
 /// An opaque type that represents a Future in the FoundationDB C API.
 pub(crate) struct FdbFuture {
     f: Option<*mut fdb::FDBFuture>,
-    task: Option<Box<futures::task::Task>>,
+    cb_active: bool,
 }
 
 impl FdbFuture {
     // `new` is marked as unsafe because it's lifetime is not well-defined.
     pub(crate) unsafe fn new(f: *mut fdb::FDBFuture) -> Self {
         Self {
             f: Some(f),
-            task: None,
+            cb_active: false,
         }
     }
 }
@@ -56,14 +56,14 @@ impl futures::Future for FdbFuture {
     fn poll(&mut self) -> std::result::Result<Async<Self::Item>, Self::Error> {
         let f = self.f.expect("cannot poll after resolve");
 
-        if self.task.is_none() {
+        if !self.cb_active {
             let task = futures::task::current();
             let task = Box::new(task);
-            let task_ptr = task.as_ref() as *const _;
+            let task_ptr = Box::into_raw(task);
             unsafe {
                 fdb::fdb_future_set_callback(f, Some(fdb_future_callback), task_ptr as *mut _);
             }
-            self.task = Some(task);
+            self.cb_active = true;
 
             return Ok(Async::NotReady);
         }
@@ -87,8 +87,7 @@ extern "C" fn fdb_future_callback(
     _f: *mut fdb::FDBFuture,
     callback_parameter: *mut ::std::os::raw::c_void,
 ) {
-    let task: *const futures::task::Task = callback_parameter as *const _;
-    let task: &futures::task::Task = unsafe { &*task };
+    let task = unsafe { Box::from_raw(callback_parameter as *mut futures::task::Task) };
     task.notify();
 }
 

foundationdb/tests/future.rs

@@ -0,0 +1,58 @@
+// Copyright 2019 foundationdb-rs developers, https://github.com/bluejekyll/foundationdb-rs/graphs/contributors
+//
+// Licensed under the Apache License, Version 2.0, <LICENSE-APACHE or
+// http://apache.org/licenses/LICENSE-2.0> or the MIT license <LICENSE-MIT or
+// http://opensource.org/licenses/MIT>, at your option. This file may not be
+// copied, modified, or distributed except according to those terms.
+
+extern crate foundationdb;
+extern crate futures;
+#[macro_use]
+extern crate lazy_static;
+
+use crate::error::Result;
+use foundationdb::transaction::TrxGet;
+use foundationdb::*;
+use futures::prelude::*;
+
+mod common;
+
+struct AbortingFuture {
+    inner: TrxGet,
+    polled: bool,
+}
+
+impl Future for AbortingFuture {
+    type Item = ();
+    type Error = Error;
+
+    fn poll(&mut self) -> Result<Async<Self::Item>> {
+        // poll once only
+        if !self.polled {
+            self.polled = true;
+            let _ = self.inner.poll();
+        }
+
+        Ok(Async::Ready(()))
+    }
+}
+
+#[test]
+// dropping a future while it's in the pending state should not crash
+fn test_future_discard() {
+    common::setup_static();
+
+    let db = Cluster::new(foundationdb::default_config_path())
+        .and_then(|cluster| cluster.create_database())
+        .wait()
+        .unwrap();
+
+    for _i in 0..=1000 {
+        db.transact(|trx| AbortingFuture {
+            inner: trx.get(b"key", false),
+            polled: false,
+        })
+        .wait()
+        .unwrap();
+    }
+}