Skip to content

Commit a84a45d

Browse files
committed
Fix deadlock when pipelining statements.
When executing statements in parallel there is a race where we prepare the type info queries multiple times, and so insert into the type info caches multiple times. This resulted in any existing cached `Statement` to be dropped, running its destructor which attempts to take out the state lock that is already being held, resulting in a deadlock. Fixes rust-postgres#772.
1 parent 3073435 commit a84a45d

1 file changed

Lines changed: 42 additions & 3 deletions

File tree

tokio-postgres/src/client.rs

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,17 @@ impl Responses {
5555
}
5656

5757
struct State {
58+
/// A cached prepared statement for basic information for a type from its
59+
/// OID. Corresponds to [TYPEINFO_QUERY](prepare::TYPEINFO_QUERY) (or its
60+
/// fallback).
5861
typeinfo: Option<Statement>,
62+
/// A cached prepared statement for getting information for a composite type
63+
/// from its OID. Corresponds to
64+
/// [TYPEINFO_QUERY](prepare::TYPEINFO_COMPOSITE_QUERY).
5965
typeinfo_composite: Option<Statement>,
66+
/// A cached prepared statement for getting information for a composite type
67+
/// from its OID. Corresponds to
68+
/// [TYPEINFO_QUERY](prepare::TYPEINFO_COMPOSITE_QUERY) (or its fallback).
6069
typeinfo_enum: Option<Statement>,
6170
types: HashMap<Oid, Type>,
6271
buf: BytesMut,
@@ -86,23 +95,53 @@ impl InnerClient {
8695
}
8796

8897
pub fn set_typeinfo(&self, statement: &Statement) {
89-
self.state.lock().typeinfo = Some(statement.clone());
98+
// We only insert the statement if there isn't already a cached
99+
// statement (this is safe as they are prepared statements for the same
100+
// query).
101+
//
102+
// Note: We need to be sure that we don't drop a Statement while holding
103+
// the state lock as its drop handling will call `with_buf`, which tries
104+
// to take the lock.
105+
let mut state = self.state.lock();
106+
if state.typeinfo.is_none() {
107+
state.typeinfo = Some(statement.clone());
108+
}
90109
}
91110

92111
pub fn typeinfo_composite(&self) -> Option<Statement> {
93112
self.state.lock().typeinfo_composite.clone()
94113
}
95114

96115
pub fn set_typeinfo_composite(&self, statement: &Statement) {
97-
self.state.lock().typeinfo_composite = Some(statement.clone());
116+
// We only insert the statement if there isn't already a cached
117+
// statement (this is safe as they are prepared statements for the same
118+
// query).
119+
//
120+
// Note: We need to be sure that we don't drop a Statement while holding
121+
// the state lock as its drop handling will call `with_buf`, which tries
122+
// to take the lock.
123+
let mut state = self.state.lock();
124+
if state.typeinfo_composite.is_none() {
125+
state.typeinfo_composite = Some(statement.clone());
126+
}
98127
}
99128

100129
pub fn typeinfo_enum(&self) -> Option<Statement> {
101130
self.state.lock().typeinfo_enum.clone()
102131
}
103132

104133
pub fn set_typeinfo_enum(&self, statement: &Statement) {
105-
self.state.lock().typeinfo_enum = Some(statement.clone());
134+
// We only insert the statement if there isn't already a cached
135+
// statement (this is safe as they are prepared statements for the same
136+
// query).
137+
//
138+
// Note: We need to be sure that we don't drop a Statement while holding
139+
// the state lock as its drop handling will call `with_buf`, which tries
140+
// to take the lock.
141+
let mut state = self.state.lock();
142+
if state.typeinfo_enum.is_none() {
143+
state.typeinfo_enum = Some(statement.clone());
144+
}
106145
}
107146

108147
pub fn type_(&self, oid: Oid) -> Option<Type> {

0 commit comments

Comments
 (0)