Skip to content

Commit bbdd78b

Browse files
committed
refactor(postgres): new #[sqlx::test] architecture
1 parent 43a9760 commit bbdd78b

8 files changed

Lines changed: 219 additions & 90 deletions

File tree

sqlx-core/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,8 @@ hashbrown = "0.16.0"
105105

106106
futures-intrusive = "0.5.0"
107107

108+
pin-project-lite = "0.2.17"
109+
108110
thiserror.workspace = true
109111

110112
[dev-dependencies.sqlx]

sqlx-core/src/pool/connection.rs

Lines changed: 18 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,15 @@ impl<DB: Database> PoolConnection<DB> {
127127
self.live.take().expect(EXPECT_MSG)
128128
}
129129

130+
/// Release the connection.
131+
///
132+
/// If the connection is to be closed, close it and run pool maintenance.
133+
///
130134
/// Test the connection to make sure it is still live before returning it to the pool.
131135
///
132136
/// This effectively runs the drop handler eagerly instead of spawning a task to do it.
133137
#[doc(hidden)]
134-
pub fn return_to_pool(&mut self) -> impl Future<Output = ()> + Send + 'static {
138+
pub fn release(&mut self) -> impl Future<Output = ()> + Send + 'static {
135139
// float the connection in the pool before we move into the task
136140
// in case the returned `Future` isn't executed, like if it's spawned into a dying runtime
137141
// https://github.com/launchbadge/sqlx/issues/1396
@@ -141,9 +145,20 @@ impl<DB: Database> PoolConnection<DB> {
141145

142146
let pool = self.pool.clone();
143147

148+
let close_on_drop = self.close_on_drop;
149+
144150
async move {
145151
let returned_to_pool = if let Some(floating) = floating {
146-
floating.return_to_pool().await
152+
if close_on_drop {
153+
// Don't hold the connection forever if it hangs while trying to close
154+
crate::rt::timeout(CLOSE_ON_DROP_TIMEOUT, floating.close())
155+
.await
156+
.ok();
157+
158+
false
159+
} else {
160+
floating.return_to_pool().await
161+
}
147162
} else {
148163
false
149164
};
@@ -153,27 +168,6 @@ impl<DB: Database> PoolConnection<DB> {
153168
}
154169
}
155170
}
156-
157-
fn take_and_close(&mut self) -> impl Future<Output = ()> + Send + 'static {
158-
// float the connection in the pool before we move into the task
159-
// in case the returned `Future` isn't executed, like if it's spawned into a dying runtime
160-
// https://github.com/launchbadge/sqlx/issues/1396
161-
// Type hints seem to be broken by `Option` combinators in IntelliJ Rust right now (6/22).
162-
let floating = self.live.take().map(|live| live.float(self.pool.clone()));
163-
164-
let pool = self.pool.clone();
165-
166-
async move {
167-
if let Some(floating) = floating {
168-
// Don't hold the connection forever if it hangs while trying to close
169-
crate::rt::timeout(CLOSE_ON_DROP_TIMEOUT, floating.close())
170-
.await
171-
.ok();
172-
}
173-
174-
pool.min_connections_maintenance(None).await;
175-
}
176-
}
177171
}
178172

179173
impl<'c, DB: Database> crate::acquire::Acquire<'c> for &'c mut PoolConnection<DB> {
@@ -198,14 +192,9 @@ impl<'c, DB: Database> crate::acquire::Acquire<'c> for &'c mut PoolConnection<DB
198192
/// Returns the connection to the [`Pool`][crate::pool::Pool] it was checked-out from.
199193
impl<DB: Database> Drop for PoolConnection<DB> {
200194
fn drop(&mut self) {
201-
if self.close_on_drop {
202-
crate::rt::spawn(self.take_and_close());
203-
return;
204-
}
205-
206195
// We still need to spawn a task to maintain `min_connections`.
207196
if self.live.is_some() || self.pool.options.min_connections > 0 {
208-
crate::rt::spawn(self.return_to_pool());
197+
crate::rt::spawn(self.release());
209198
}
210199
}
211200
}

sqlx-core/src/sync.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ impl<T> AsyncOnceCell<T> {
225225
} else if #[cfg(feature = "_rt-async-io")] {
226226
Self { inner: async_lock::OnceCell::new() }
227227
} else {
228-
crate::rt::missing_rt(());
228+
Self { phantom: std::marker::PhantomData }
229229
}
230230
}
231231
}
@@ -237,7 +237,7 @@ impl<T> AsyncOnceCell<T> {
237237
} else if #[cfg(feature = "_rt-async-io")] {
238238
Self { inner: async_lock::OnceCell::new() }
239239
} else {
240-
crate::rt::missing_rt(());
240+
Self { phantom: std::marker::PhantomData }
241241
}
242242
}
243243
}
@@ -255,6 +255,16 @@ impl<T> AsyncOnceCell<T> {
255255
}
256256
}
257257
}
258+
259+
pub fn get(&self) -> Option<&T> {
260+
cfg_if! {
261+
if #[cfg(any(feature = "_rt-tokio", feature = "_rt-async-io"))] {
262+
self.inner.get()
263+
} else {
264+
crate::rt::missing_rt(())
265+
}
266+
}
267+
}
258268
}
259269

260270
impl<T> Default for AsyncOnceCell<T> {

sqlx-core/src/testing/mod.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ use crate::pool::{Pool, PoolConnection, PoolOptions};
1515
mod fixtures;
1616
mod pool;
1717

18+
pub use pool::{TestMasterConnection, TestMasterPool};
19+
1820
pub trait TestSupport: Database {
1921
/// Get parameters to construct a `Pool` suitable for testing.
2022
///
@@ -67,7 +69,7 @@ pub struct TestArgs {
6769
pub test_path: &'static str,
6870
pub migrator: Option<&'static Migrator>,
6971
pub fixtures: &'static [TestFixture],
70-
pub max_connections: usize,
72+
pub max_connections: u32,
7173
}
7274

7375
pub trait TestFn {
@@ -172,7 +174,7 @@ impl TestArgs {
172174
self.fixtures = fixtures;
173175
}
174176

175-
pub fn max_connections(&mut self, max_connections: usize) {
177+
pub fn max_connections(&mut self, max_connections: u32) {
176178
self.max_connections = max_connections;
177179
}
178180
}

sqlx-core/src/testing/pool.rs

Lines changed: 167 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,184 @@
1-
use std::rc::Weak;
1+
use crate::connection::Connection;
22
use crate::database::Database;
3-
use crate::pool::Pool;
3+
use crate::pool::{Pool, PoolConnection, PoolOptions};
44
use crate::sync::AsyncOnceCell;
5+
use cfg_if::cfg_if;
6+
use pin_project_lite::pin_project;
7+
use std::future::Future;
8+
use std::ops::{Deref, DerefMut};
9+
use std::pin::Pin;
10+
use std::task::{Context, Poll};
511

612
pub struct TestMasterPool<DB: Database> {
713
inner: AsyncOnceCell<Inner<DB>>,
814
}
915

16+
pub struct TestMasterConnection<DB: Database> {
17+
conn: PoolConnection<DB>,
18+
19+
#[cfg(feature = "_rt-tokio")]
20+
handle: tokio::runtime::Handle,
21+
}
22+
1023
struct Inner<DB: Database> {
1124
pool: Pool<DB>,
1225

1326
#[cfg(feature = "_rt-tokio")]
14-
27+
handle: tokio::runtime::Handle,
1528
}
1629

30+
macro_rules! poll_with_handle(
31+
($handle:expr, $fut:expr) => {
32+
PollWithHandle {
33+
fut: $fut,
34+
#[cfg(feature = "_rt-tokio")]
35+
handle: &$handle,
36+
#[cfg(not(feature = "_rt-tokio"))]
37+
_marker: std::marker::PhantomData,
38+
}
39+
}
40+
);
1741

1842
impl<DB: Database> TestMasterPool<DB> {
43+
pub const fn new() -> Self {
44+
TestMasterPool {
45+
inner: AsyncOnceCell::const_new(),
46+
}
47+
}
48+
49+
pub async fn connect(
50+
&self,
51+
opts: &<DB::Connection as Connection>::Options,
52+
) -> crate::Result<TestMasterConnection<DB>> {
53+
self.inner
54+
.get_or_try_init::<_, _, crate::Error>(|| {
55+
let opts = opts.clone();
56+
57+
async move {
58+
#[cfg(feature = "_rt-tokio")]
59+
let handle = spawn_test_runtime();
60+
61+
// Ensure this pool is linked to our master runtime so it can survive an individual
62+
// test runtime shutting down.
63+
let pool = poll_with_handle!(
64+
handle,
65+
PoolOptions::new()
66+
// Tests don't need a master connection for very long
67+
.max_connections(1)
68+
.test_before_acquire(false)
69+
.connect_with(opts)
70+
)
71+
.await?;
72+
73+
Ok(Inner {
74+
pool,
75+
#[cfg(feature = "_rt-tokio")]
76+
handle,
77+
})
78+
}
79+
})
80+
.await?
81+
.acquire()
82+
.await
83+
}
84+
85+
/// # Panics
86+
/// If [`Self::connect()`] has not already completed successfully.
87+
pub async fn acquire(&self) -> crate::Result<TestMasterConnection<DB>> {
88+
self.inner
89+
.get()
90+
.expect("`TestMasterPool::connect()` has not been called")
91+
.acquire()
92+
.await
93+
}
94+
}
95+
96+
impl<DB: Database> Deref for TestMasterConnection<DB> {
97+
type Target = PoolConnection<DB>;
98+
99+
fn deref(&self) -> &Self::Target {
100+
&self.conn
101+
}
102+
}
103+
104+
impl<DB: Database> DerefMut for TestMasterConnection<DB> {
105+
fn deref_mut(&mut self) -> &mut Self::Target {
106+
&mut self.conn
107+
}
108+
}
109+
110+
impl<DB: Database> Drop for TestMasterConnection<DB> {
111+
fn drop(&mut self) {
112+
cfg_if!(
113+
if #[cfg(feature = "_rt-tokio")] {
114+
self.handle.spawn(self.conn.release());
115+
} else {
116+
crate::rt::spawn(self.conn.release());
117+
}
118+
);
119+
}
120+
}
121+
122+
impl<DB: Database> Inner<DB> {
123+
async fn acquire(&self) -> crate::Result<TestMasterConnection<DB>> {
124+
Ok(TestMasterConnection {
125+
// Ostensibly we only need to enter the runtime if the connection isn't already established
126+
conn: poll_with_handle!(self.handle, self.pool.acquire()).await?,
127+
#[cfg(feature = "_rt-tokio")]
128+
handle: self.handle.clone(),
129+
})
130+
}
131+
}
132+
133+
// It's likely not advisable to hold an `EnterGuard` across an `.await` point,
134+
// so we need to define an adapter that only enters the alternate runtime when it's polled.
135+
#[cfg(feature = "_rt-tokio")]
136+
pin_project! {
137+
struct PollWithHandle<'a, F> {
138+
#[pin]
139+
fut: F,
140+
handle: &'a tokio::runtime::Handle,
141+
}
142+
}
143+
144+
#[cfg(not(feature = "_rt-tokio"))]
145+
pin_project! {
146+
struct PollWithHandle<'a, F> {
147+
#[pin]
148+
fut: F,
149+
_marker: std::marker::PhantomData<&'a ()>,
150+
}
151+
}
152+
153+
impl<F: Future> Future for PollWithHandle<'_, F> {
154+
type Output = F::Output;
155+
156+
fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
157+
let this = self.project();
158+
159+
#[cfg(feature = "_rt-tokio")]
160+
let _guard = this.handle.enter();
161+
162+
this.fut.poll(cx)
163+
}
164+
}
165+
166+
#[cfg(feature = "_rt-tokio")]
167+
fn spawn_test_runtime() -> tokio::runtime::Handle {
168+
// Instead of forcing the `rt-multi-thread` feature on,
169+
// we just run a current-thread runtime in a background thread that we ourselves spawn.
170+
let rt = tokio::runtime::Builder::new_current_thread()
171+
.name("sqlx-test-master-pool")
172+
.enable_all()
173+
.build()
174+
.expect("failed to spawn master runtime");
175+
176+
let handle = rt.handle().clone();
177+
178+
std::thread::Builder::new()
179+
.name("sqlx-test-master-pool".into())
180+
.spawn(move || rt.block_on(std::future::pending::<()>()))
181+
.expect("failed to spawn thread for master runtime");
19182

183+
handle
20184
}

sqlx-postgres/src/listener.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ impl Drop for PgListener {
367367
// inline the drop handler from `PoolConnection` so it doesn't try to spawn another task
368368
// otherwise, it may trigger a panic if this task is dropped because the runtime is going away:
369369
// https://github.com/launchbadge/sqlx/issues/1389
370-
conn.return_to_pool().await;
370+
conn.release().await;
371371
};
372372

373373
// Unregister any listeners before returning the connection to the pool.

0 commit comments

Comments
 (0)