Concurrent DROP TABLESPACE can miss a shared dependency
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
This thread has been committed, so CI has stopped here. Anything below is the last result it produced.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253610psql -h localhost -U postgresBuilt from patchset v3 (message #3), September 09, 2026 at 02:14 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253610_3 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253610_3 && git checkout t253610_3Patchset v3 (message #3) is on t253610_3
Hi,
I found a race between DROP TABLESPACE and a concurrent command that adds a
shared dependency on the tablespace. Both commands can succeed, leaving an
object whose pg_class.reltablespace and pg_shdepend entries refer to a
tablespace that no longer exists.
One way to reproduce this is:
1. One session updates the pg_tablespace row and keeps the transaction open.
2. A second session runs DROP TABLESPACE. It finds no dependencies, then
waits while deleting the pg_tablespace tuple.
3. A third session creates a partitioned table in the tablespace and commits.
4. The first session aborts, allowing DROP TABLESPACE to finish using the
result of its earlier dependency check.
ISTM the race is possible because shdepAddDependency() takes an
AccessShareLock on the referenced shared object and rechecks that it still
exists, but DropTableSpace() calls checkSharedDependencies() without first
taking the corresponding conflicting lock. DropRole() appears to follow
that protocol already.
For a fix, my first thought was to have DROP TABLESPACE take an
AccessExclusiveLock before checking its shared dependencies. However,
doing only that seems to introduce a lock-order problem with commands that
update pg_tablespace without first locking the tablespace. Such a command
can hold the catalog tuple while DROP TABLESPACE holds the object lock, and
then try to acquire the object lock itself when the transaction records a
tablespace dependency.
I went through the paths that update pg_tablespace, and I think ALTER
TABLESPACE RENAME/SET, DROP OWNED, and REASSIGN OWNED need to acquire an
AccessShareLock before updating the catalog tuple. Direct GRANT, REVOKE,
and ALTER OWNER appear to acquire an object lock already. The attached
0002 contains those changes, separately from the DROP-side change in 0001.
(They likely need to be squashed once the patch looks fine).
Does this AccessExclusiveLock/AccessShareLock protocol seem like the right
way to close the race? Also, have I missed another pg_tablespace update
path that should participate in the same protocol?
Regards,
Ayush
Attachments:
t253610_1v1-0001-Prevent-orphaned-tablespace-dependencies.patchapplication/octet-stream; name=v1-0001-Prevent-orphaned-tablespace-dependencies.patchDownload+108-1
v1-0002-Avoid-deadlocks-with-DROP-TABLESPACE.patchapplication/octet-stream; name=v1-0002-Avoid-deadlocks-with-DROP-TABLESPACE.patchDownload+122-9
On 2026-08-30 Su 4:05 PM, Ayush Tiwari wrote:
Hi,
I found a race between DROP TABLESPACE and a concurrent command that adds a
shared dependency on the tablespace. Both commands can succeed, leaving an
object whose pg_class.reltablespace and pg_shdepend entries refer to a
tablespace that no longer exists.One way to reproduce this is:
1. One session updates the pg_tablespace row and keeps the transaction open.
2. A second session runs DROP TABLESPACE. It finds no dependencies, then
waits while deleting the pg_tablespace tuple.
3. A third session creates a partitioned table in the tablespace and commits.
4. The first session aborts, allowing DROP TABLESPACE to finish using the
result of its earlier dependency check.ISTM the race is possible because shdepAddDependency() takes an
AccessShareLock on the referenced shared object and rechecks that it still
exists, but DropTableSpace() calls checkSharedDependencies() without first
taking the corresponding conflicting lock. DropRole() appears to follow
that protocol already.For a fix, my first thought was to have DROP TABLESPACE take an
AccessExclusiveLock before checking its shared dependencies. However,
doing only that seems to introduce a lock-order problem with commands that
update pg_tablespace without first locking the tablespace. Such a command
can hold the catalog tuple while DROP TABLESPACE holds the object lock, and
then try to acquire the object lock itself when the transaction records a
tablespace dependency.I went through the paths that update pg_tablespace, and I think ALTER
TABLESPACE RENAME/SET, DROP OWNED, and REASSIGN OWNED need to acquire an
AccessShareLock before updating the catalog tuple. Direct GRANT, REVOKE,
and ALTER OWNER appear to acquire an object lock already. The attached
0002 contains those changes, separately from the DROP-side change in 0001.
(They likely need to be squashed once the patch looks fine).Does this AccessExclusiveLock/AccessShareLock protocol seem like the right
way to close the race? Also, have I missed another pg_tablespace update
path that should participate in the same protocol?
Hi, I encountered this while working on cleaning up the ddl patches.
0002's deadlock analysis is correct. DROP
TABLESPACE takes the AccessExclusiveLock and then blocks in
CatalogTupleDelete on the uncommitted ALTER's row, while that same
transaction's later CREATE TABLE ... TABLESPACE blocks on the
AccessShareLock DROP already holds -- a real deadlock that 0001
introduces and 0002 closes by taking the AccessShareLock before
touching the catalog tuple in all four paths.
On your open question: I don't think you've missed a path. I tested
GRANT ON TABLESPACE and ALTER TABLESPACE ... OWNER TO the same way and
neither deadlocks -- DROP blocks, then correctly errors once the
transaction commits.
I think this should be applied as a single squashed commit,
(soon so I can rely on it for the fixes I mentioned).
I think it should be backpatched - all live branches have the same problem.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Hi,
On Mon, 7 Sept 2026 at 23:28, Andrew Dunstan <andrew@dunslane.net> wrote:
On 2026-08-30 Su 4:05 PM, Ayush Tiwari wrote:
Hi,
I found a race between DROP TABLESPACE and a concurrent command that adds a
shared dependency on the tablespace. Both commands can succeed, leaving an
object whose pg_class.reltablespace and pg_shdepend entries refer to a
tablespace that no longer exists.One way to reproduce this is:
1. One session updates the pg_tablespace row and keeps the transaction open.
2. A second session runs DROP TABLESPACE. It finds no dependencies, then
waits while deleting the pg_tablespace tuple.
3. A third session creates a partitioned table in the tablespace and commits.
4. The first session aborts, allowing DROP TABLESPACE to finish using the
result of its earlier dependency check.ISTM the race is possible because shdepAddDependency() takes an
AccessShareLock on the referenced shared object and rechecks that it still
exists, but DropTableSpace() calls checkSharedDependencies() without first
taking the corresponding conflicting lock. DropRole() appears to follow
that protocol already.For a fix, my first thought was to have DROP TABLESPACE take an
AccessExclusiveLock before checking its shared dependencies. However,
doing only that seems to introduce a lock-order problem with commands that
update pg_tablespace without first locking the tablespace. Such a command
can hold the catalog tuple while DROP TABLESPACE holds the object lock, and
then try to acquire the object lock itself when the transaction records a
tablespace dependency.I went through the paths that update pg_tablespace, and I think ALTER
TABLESPACE RENAME/SET, DROP OWNED, and REASSIGN OWNED need to acquire an
AccessShareLock before updating the catalog tuple. Direct GRANT, REVOKE,
and ALTER OWNER appear to acquire an object lock already. The attached
0002 contains those changes, separately from the DROP-side change in 0001.
(They likely need to be squashed once the patch looks fine).Does this AccessExclusiveLock/AccessShareLock protocol seem like the right
way to close the race? Also, have I missed another pg_tablespace update
path that should participate in the same protocol?Hi, I encountered this while working on cleaning up the ddl patches.
0002's deadlock analysis is correct. DROP
TABLESPACE takes the AccessExclusiveLock and then blocks in
CatalogTupleDelete on the uncommitted ALTER's row, while that same
transaction's later CREATE TABLE ... TABLESPACE blocks on the
AccessShareLock DROP already holds -- a real deadlock that 0001
introduces and 0002 closes by taking the AccessShareLock before
touching the catalog tuple in all four paths.On your open question: I don't think you've missed a path. I tested
GRANT ON TABLESPACE and ALTER TABLESPACE ... OWNER TO the same way and
neither deadlocks -- DROP blocks, then correctly errors once the
transaction commits.
Thanks for confirming the deadlock analysis and for checking the GRANT and
ALTER OWNER paths.
I think this should be applied as a single squashed commit,
(soon so I can rely on it for the fixes I mentioned).
I have squashed the two changes into the attached v2.
I think it should be backpatched - all live branches have the same problem.
Agreed that this should be backpatched to all live branches.
Regards,
Ayush
On Tue, Sep 08, 2026 at 07:28:51PM +0530, Ayush Tiwari wrote:
On Mon, 7 Sept 2026 at 23:28, Andrew Dunstan <andrew@dunslane.net> wrote:
I think this should be applied as a single squashed commit,
(soon so I can rely on it for the fixes I mentioned).
Yeah, a backpatch seems adapted. That's a nasty problem. It's
unlikely going to be hit in practice, but we can say that for a bunch
of stuff that gets backpatched.
I think it should be backpatched - all live branches have the same problem.
Agreed that this should be backpatched to all live branches.
Andrew, are you planning to work on it? I did not spend cycles
looking at the details and the command patterns involved, on the
premise that you would handle that. Saying that, the premise of the
patch feels sound with a few more additional shared locks to prevent
the concurrent manipulations.
--
Michael
On 2026-09-08 Tu 7:25 PM, Michael Paquier wrote:
On Tue, Sep 08, 2026 at 07:28:51PM +0530, Ayush Tiwari wrote:
On Mon, 7 Sept 2026 at 23:28, Andrew Dunstan <andrew@dunslane.net> wrote:
I think this should be applied as a single squashed commit,
(soon so I can rely on it for the fixes I mentioned).Yeah, a backpatch seems adapted. That's a nasty problem. It's
unlikely going to be hit in practice, but we can say that for a bunch
of stuff that gets backpatched.I think it should be backpatched - all live branches have the same problem.
Agreed that this should be backpatched to all live branches.
Andrew, are you planning to work on it? I did not spend cycles
looking at the details and the command patterns involved, on the
premise that you would handle that. Saying that, the premise of the
patch feels sound with a few more additional shared locks to prevent
the concurrent manipulations.
Thanks for looking.
Yes, planning to work on it today.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
On 2026-09-09 We 7:39 AM, Andrew Dunstan wrote:
On 2026-09-08 Tu 7:25 PM, Michael Paquier wrote:
On Tue, Sep 08, 2026 at 07:28:51PM +0530, Ayush Tiwari wrote:
On Mon, 7 Sept 2026 at 23:28, Andrew Dunstan <andrew@dunslane.net>
wrote:I think this should be applied as a single squashed commit,
(soon so I can rely on it for the fixes I mentioned).Yeah, a backpatch seems adapted. That's a nasty problem. It's
unlikely going to be hit in practice, but we can say that for a bunch
of stuff that gets backpatched.I think it should be backpatched - all live branches have the same
problem.Agreed that this should be backpatched to all live branches.
Andrew, are you planning to work on it? I did not spend cycles
looking at the details and the command patterns involved, on the
premise that you would handle that. Saying that, the premise of the
patch feels sound with a few more additional shared locks to prevent
the concurrent manipulations.Thanks for looking.
Yes, planning to work on it today.
Pushed, thanks all.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com