Skip to content

Commit c53f7f6

Browse files
author
Vincent Rouillé
committed
Fix even run can lead to UB
Restore 0.4 recommanded boot sequence but wrapped in an unsafe block.
1 parent 7b5d67e commit c53f7f6

File tree

13 files changed

+107
-124
lines changed

13 files changed

+107
-124
lines changed

foundationdb-bench/src/bin/fdb-bench.rs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -146,13 +146,12 @@ fn main() {
146146
let opt = Opt::from_args();
147147
info!("opt: {:?}", opt);
148148

149-
fdb::run(|| {
150-
let db = Arc::new(
151-
futures::executor::block_on(fdb::Database::new_compat(None))
152-
.expect("failed to get database"),
153-
);
154-
155-
let bench = Bench { db, opt };
156-
bench.run();
157-
});
149+
let _guard = unsafe { foundationdb::boot() };
150+
let db = Arc::new(
151+
futures::executor::block_on(fdb::Database::new_compat(None))
152+
.expect("failed to get database"),
153+
);
154+
155+
let bench = Bench { db, opt };
156+
bench.run();
158157
}

foundationdb-bindingtester/src/main.rs

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1582,23 +1582,22 @@ fn main() {
15821582
"Starting rust bindingtester with api_version {}",
15831583
api_version
15841584
);
1585-
api::FdbApiBuilder::default()
1585+
let builder = api::FdbApiBuilder::default()
15861586
.set_runtime_version(api_version)
15871587
.build()
1588-
.expect("failed to initialize FoundationDB API")
1589-
.run(|| {
1590-
let db = Arc::new(
1591-
futures::executor::block_on(fdb::Database::new_compat(cluster_path))
1592-
.expect("failed to get database"),
1593-
);
1594-
1595-
let mut sm = StackMachine::new(&db, Bytes::from(prefix.to_owned().into_bytes()));
1596-
futures::executor::block_on(sm.run(db)).unwrap();
1597-
sm.join();
1598-
1599-
info!("Closing...");
1600-
})
1601-
.expect("failed to initialize FoundationDB network thread");
1588+
.expect("failed to initialize FoundationDB API");
1589+
let _network = unsafe { builder.boot() };
1590+
1591+
let db = Arc::new(
1592+
futures::executor::block_on(fdb::Database::new_compat(cluster_path))
1593+
.expect("failed to get database"),
1594+
);
1595+
1596+
let mut sm = StackMachine::new(&db, Bytes::from(prefix.to_owned().into_bytes()));
1597+
futures::executor::block_on(sm.run(db)).unwrap();
1598+
sm.join();
1599+
1600+
info!("Closing...");
16021601

16031602
info!("Done.");
16041603
}

foundationdb/examples/class-scheduling.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -364,11 +364,10 @@ async fn run_sim(db: &Database, students: usize, ops_per_student: usize) {
364364
}
365365

366366
fn main() {
367-
fdb::run(|| {
368-
let db = futures::executor::block_on(fdb::Database::new_compat(None))
369-
.expect("failed to get database");
370-
futures::executor::block_on(init(&db, &*ALL_CLASSES));
371-
println!("Initialized");
372-
futures::executor::block_on(run_sim(&db, 10, 10));
373-
});
367+
let _guard = unsafe { fdb::boot() };
368+
let db = futures::executor::block_on(fdb::Database::new_compat(None))
369+
.expect("failed to get database");
370+
futures::executor::block_on(init(&db, &*ALL_CLASSES));
371+
println!("Initialized");
372+
futures::executor::block_on(run_sim(&db, 10, 10));
374373
}

foundationdb/src/api.rs

Lines changed: 5 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,9 @@ impl Default for FdbApiBuilder {
9090
/// use foundationdb::api::FdbApiBuilder;
9191
///
9292
/// let network_builder = FdbApiBuilder::default().build().expect("fdb api initialized");
93-
/// network_builder.run(|| {
94-
/// // do some work with foundationDB
95-
/// }).expect("fdb network to run");
93+
/// let guard = unsafe { network_builder.boot() };
94+
/// // do some work with foundationDB
95+
/// drop(guard);
9696
/// ```
9797
pub struct NetworkBuilder {
9898
_private: (),
@@ -142,30 +142,6 @@ impl NetworkBuilder {
142142
Ok((NetworkRunner { cond: cond.clone() }, NetworkWait { cond }))
143143
}
144144

145-
/// Execute `f` with the FoundationDB Client API ready, this can only be called once per process.
146-
/// Once this method returns, the FoundationDB Client API is stopped and cannot be restarted.
147-
///
148-
/// # Examples
149-
///
150-
/// ```rust
151-
/// use foundationdb::api::FdbApiBuilder;
152-
///
153-
/// let network_builder = FdbApiBuilder::default().build().expect("fdb api initialized");
154-
/// network_builder.run(|| {
155-
/// // do some interesting things with the API...
156-
/// });
157-
/// ```
158-
pub fn run<T>(self, f: impl FnOnce() -> T) -> FdbResult<T> {
159-
// Safe because the returned object is dropped before this function returns
160-
let must_drop = unsafe { self.boot()? };
161-
162-
let t = f();
163-
164-
drop(must_drop);
165-
166-
Ok(t)
167-
}
168-
169145
/// Initialize the FoundationDB Client API, this can only be called once per process.
170146
///
171147
/// # Returns
@@ -186,7 +162,7 @@ impl NetworkBuilder {
186162
/// use foundationdb::api::FdbApiBuilder;
187163
///
188164
/// let network_builder = FdbApiBuilder::default().build().expect("fdb api initialized");
189-
/// let network = unsafe { network_builder.boot() }.expect("fdb network running");
165+
/// let network = unsafe { network_builder.boot() };
190166
/// // do some interesting things with the API...
191167
/// drop(network);
192168
/// ```
@@ -197,7 +173,7 @@ impl NetworkBuilder {
197173
/// #[tokio::main]
198174
/// async fn main() {
199175
/// let network_builder = FdbApiBuilder::default().build().expect("fdb api initialized");
200-
/// let network = unsafe { network_builder.boot() }.expect("fdb network running");
176+
/// let network = unsafe { network_builder.boot() };
201177
/// // do some interesting things with the API...
202178
/// drop(network);
203179
/// }

foundationdb/src/lib.rs

Lines changed: 25 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -18,28 +18,28 @@
1818
//! - Ubuntu Linux (this may work on the Linux subsystem for Windows as well)
1919
//!
2020
//! ```console
21-
//! $> curl -O https://www.foundationdb.org/downloads/6.1.12/ubuntu/installers/foundationdb-clients_6.1.12-1_amd64.deb
22-
//! $> curl -O https://www.foundationdb.org/downloads/6.1.12/ubuntu/installers/foundationdb-server_6.1.12-1_amd64.deb
23-
//! $> sudo dpkg -i foundationdb-clients_6.1.12-1_amd64.deb
24-
//! $> sudo dpkg -i foundationdb-server_6.1.12-1_amd64.deb
21+
//! $> curl -O https://www.foundationdb.org/downloads/6.2.25/ubuntu/installers/foundationdb-clients_6.2.25-1_amd64.deb
22+
//! $> curl -O https://www.foundationdb.org/downloads/6.2.25/ubuntu/installers/foundationdb-server_6.2.25-1_amd64.deb
23+
//! $> sudo dpkg -i foundationdb-clients_6.2.25-1_amd64.deb
24+
//! $> sudo dpkg -i foundationdb-server_6.2.25-1_amd64.deb
2525
//! ```
2626
//!
2727
//! - macOS
2828
//!
2929
//! ```console
30-
//! $> curl -O https://www.foundationdb.org/downloads/6.1.12/macOS/installers/FoundationDB-6.1.12.pkg
31-
//! $> sudo installer -pkg FoundationDB-6.1.12.pkg -target /
30+
//! $> curl -O https://www.foundationdb.org/downloads/6.2.25/macOS/installers/FoundationDB-6.2.25.pkg
31+
//! $> sudo installer -pkg FoundationDB-6.2.25.pkg -target /
3232
//! ```
3333
//!
3434
//! - Windows
3535
//!
36-
//! Install [foundationdb-6.1.12-x64.msi](https://www.foundationdb.org/downloads/6.1.12/windows/installers/foundationdb-6.1.12-x64.msi)
36+
//! Install [foundationdb-6.2.25-x64.msi](https://www.foundationdb.org/downloads/6.2.25/windows/installers/foundationdb-6.2.25-x64.msi)
3737
//!
3838
//! ## Add dependencies on foundationdb-rs
3939
//!
4040
//! ```toml
4141
//! [dependencies]
42-
//! foundationdb = "0.4"
42+
//! foundationdb = "0.5"
4343
//! futures = "0.3"
4444
//! ```
4545
//!
@@ -70,9 +70,23 @@
7070
//! Ok(())
7171
//! }
7272
//!
73-
//! foundationdb::run(|| {
74-
//! futures::executor::block_on(async_main()).expect("failed to run");
75-
//! });
73+
//! // Safe because drop is called before the program exits
74+
//! let network = unsafe { foundationdb::boot() };
75+
//! futures::executor::block_on(async_main()).expect("failed to run");
76+
//! drop(network);
77+
//! ```
78+
//!
79+
//! ```rust
80+
//! #[tokio::main]
81+
//! async fn main() {
82+
//! // Safe because drop is called before the program exits
83+
//! let network = unsafe { foundationdb::boot() };
84+
//!
85+
//! // Have fun with the FDB API
86+
//!
87+
//! // shutdown the client
88+
//! drop(network);
89+
//! }
7690
//! ```
7791
//!
7892
//! ## API stability
@@ -104,23 +118,6 @@ pub use crate::error::FdbResult;
104118
pub use crate::keyselector::*;
105119
pub use crate::transaction::*;
106120

107-
/// Execute `f` with the FoundationDB Client API ready, this can only be called once per process.
108-
///
109-
/// # Examples
110-
///
111-
/// ```rust
112-
/// foundationdb::run(|| {
113-
/// // do some interesting things with the API...
114-
/// });
115-
/// ```
116-
pub fn run<T>(f: impl FnOnce() -> T) -> T {
117-
api::FdbApiBuilder::default()
118-
.build()
119-
.expect("foundationdb API to be initialized")
120-
.run(f)
121-
.expect("foundationdb network to be setup")
122-
}
123-
124121
/// Initialize the FoundationDB Client API, this can only be called once per process.
125122
///
126123
/// # Returns

foundationdb/tests/api_ub.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
use std::panic;
2+
3+
#[test]
4+
fn test_run() {
5+
panic::set_hook(Box::new(|_| {}));
6+
let mut db = None;
7+
let result = panic::catch_unwind(panic::AssertUnwindSafe(|| {
8+
// Run the foundationdb client API
9+
let _drop_me = unsafe { foundationdb::boot() };
10+
db = Some(foundationdb::Database::default().unwrap());
11+
// Try to escape via unwind
12+
panic!("UNWIND!")
13+
}));
14+
assert!(result.is_err());
15+
let trx = db.unwrap().create_trx().unwrap();
16+
let _err = futures::executor::block_on(trx.get_read_version()).unwrap_err();
17+
}

foundationdb/tests/atomic.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ mod common;
1212

1313
#[test]
1414
fn test_atomic() {
15-
run(|| futures::executor::block_on(test_atomic_async()).expect("failed to run"));
15+
let _guard = unsafe { foundationdb::boot() };
16+
futures::executor::block_on(test_atomic_async()).expect("failed to run");
1617
}
1718

1819
async fn atomic_add(db: &Database, key: &[u8], value: i64) -> FdbResult<()> {

foundationdb/tests/future.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ where
3232

3333
#[test]
3434
fn test_future_discard() {
35-
run(|| futures::executor::block_on(test_future_discard_async()).expect("failed to run"));
35+
let _guard = unsafe { foundationdb::boot() };
36+
futures::executor::block_on(test_future_discard_async()).expect("failed to run");
3637
}
3738

3839
async fn test_future_discard_async() -> FdbResult<()> {

foundationdb/tests/get.rs

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,18 @@ mod common;
1313

1414
#[test]
1515
fn test_get() {
16-
run(|| {
17-
futures::executor::block_on(test_set_get_async()).expect("failed to run");
18-
futures::executor::block_on(test_get_multi_async()).expect("failed to run");
19-
futures::executor::block_on(test_set_conflict_async()).expect("failed to run");
20-
futures::executor::block_on(test_set_conflict_snapshot_async()).expect("failed to run");
21-
futures::executor::block_on(test_transact_async()).expect("failed to run");
22-
futures::executor::block_on(test_transact_limit()).expect("failed to run");
23-
futures::executor::block_on(test_transact_timeout()).expect("failed to run");
24-
futures::executor::block_on(test_versionstamp_async()).expect("failed to run");
25-
futures::executor::block_on(test_read_version_async()).expect("failed to run");
26-
futures::executor::block_on(test_set_read_version_async()).expect("failed to run");
27-
futures::executor::block_on(test_get_addresses_for_key_async()).expect("failed to run");
28-
});
16+
let _guard = unsafe { foundationdb::boot() };
17+
futures::executor::block_on(test_set_get_async()).expect("failed to run");
18+
futures::executor::block_on(test_get_multi_async()).expect("failed to run");
19+
futures::executor::block_on(test_set_conflict_async()).expect("failed to run");
20+
futures::executor::block_on(test_set_conflict_snapshot_async()).expect("failed to run");
21+
futures::executor::block_on(test_transact_async()).expect("failed to run");
22+
futures::executor::block_on(test_transact_limit()).expect("failed to run");
23+
futures::executor::block_on(test_transact_timeout()).expect("failed to run");
24+
futures::executor::block_on(test_versionstamp_async()).expect("failed to run");
25+
futures::executor::block_on(test_read_version_async()).expect("failed to run");
26+
futures::executor::block_on(test_set_read_version_async()).expect("failed to run");
27+
futures::executor::block_on(test_get_addresses_for_key_async()).expect("failed to run");
2928
}
3029

3130
async fn test_set_get_async() -> FdbResult<()> {

foundationdb/tests/hca.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,10 @@ mod common;
1616

1717
#[test]
1818
fn test_hca_many_sequential_allocations() {
19-
foundationdb::run(|| {
20-
futures::executor::block_on(test_hca_many_sequential_allocations_async())
21-
.expect("failed to run");
22-
futures::executor::block_on(test_hca_concurrent_allocations_async())
23-
.expect("failed to run");
24-
});
19+
let _guard = unsafe { foundationdb::boot() };
20+
futures::executor::block_on(test_hca_many_sequential_allocations_async())
21+
.expect("failed to run");
22+
futures::executor::block_on(test_hca_concurrent_allocations_async()).expect("failed to run");
2523
}
2624

2725
async fn test_hca_many_sequential_allocations_async() -> FdbResult<()> {

0 commit comments

Comments
 (0)