Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitac23b71

Browse files
committed
Fix incorrect order of database-locking operations in InitPostgres().
We should set MyProc->databaseId after acquiring the per-database lock,not beforehand. The old way risked deadlock against processes trying tocopy or delete the target database, since they would first acquire the lockand then wait for processes with matching databaseId to exit; that left awindow wherein an incoming process could set its databaseId and then blockon the lock, while the other process had the lock and waited in vain forthe incoming process to exit.CountOtherDBBackends() would time out and fail after 5 seconds, so thisjust resulted in an unexpected failure not a permanent lockup, but it'sstill annoying when it happens. A real-world example of a use-case is thatshort-duration connections to a template database should not cause CREATEDATABASE to fail.Doing it in the other order should be fine since the contract has alwaysbeen that processes searching the ProcArray for a database ID must hold therelevant per-database lock while searching. Thus, this actually removesthe former race condition that required an assumption that storing toMyProc->databaseId is atomic.It's been like this for a long time, so back-patch to all active branches.
1 parent068cfad commitac23b71

File tree

1 file changed

+29
-15
lines changed

1 file changed

+29
-15
lines changed

‎src/backend/utils/init/postinit.c

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -848,28 +848,20 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
848848
strcpy(out_dbname,dbname);
849849
}
850850

851-
/* Now we can mark our PGPROC entry with the database ID */
852-
/* (We assume this is an atomic store so no lock is needed) */
853-
MyProc->databaseId=MyDatabaseId;
854-
855-
/*
856-
* We established a catalog snapshot while reading pg_authid and/or
857-
* pg_database; but until we have set up MyDatabaseId, we won't react to
858-
* incoming sinval messages for unshared catalogs, so we won't realize it
859-
* if the snapshot has been invalidated. Assume it's no good anymore.
860-
*/
861-
InvalidateCatalogSnapshot();
862-
863851
/*
864852
* Now, take a writer's lock on the database we are trying to connect to.
865853
* If there is a concurrently running DROP DATABASE on that database, this
866854
* will block us until it finishes (and has committed its update of
867855
* pg_database).
868856
*
869857
* Note that the lock is not held long, only until the end of this startup
870-
* transaction. This is OK since we are already advertising our use of
871-
* the database in the PGPROC array; anyone trying a DROP DATABASE after
872-
* this point will see us there.
858+
* transaction. This is OK since we will advertise our use of the
859+
* database in the ProcArray before dropping the lock (in fact, that's the
860+
* next thing to do). Anyone trying a DROP DATABASE after this point will
861+
* see us in the array once they have the lock. Ordering is important for
862+
* this because we don't want to advertise ourselves as being in this
863+
* database until we have the lock; otherwise we create what amounts to a
864+
* deadlock with CountOtherDBBackends().
873865
*
874866
* Note: use of RowExclusiveLock here is reasonable because we envision
875867
* our session as being a concurrent writer of the database. If we had a
@@ -881,6 +873,28 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
881873
LockSharedObject(DatabaseRelationId,MyDatabaseId,0,
882874
RowExclusiveLock);
883875

876+
/*
877+
* Now we can mark our PGPROC entry with the database ID.
878+
*
879+
* We assume this is an atomic store so no lock is needed; though actually
880+
* things would work fine even if it weren't atomic. Anyone searching the
881+
* ProcArray for this database's ID should hold the database lock, so they
882+
* would not be executing concurrently with this store. A process looking
883+
* for another database's ID could in theory see a chance match if it read
884+
* a partially-updated databaseId value; but as long as all such searches
885+
* wait and retry, as in CountOtherDBBackends(), they will certainly see
886+
* the correct value on their next try.
887+
*/
888+
MyProc->databaseId=MyDatabaseId;
889+
890+
/*
891+
* We established a catalog snapshot while reading pg_authid and/or
892+
* pg_database; but until we have set up MyDatabaseId, we won't react to
893+
* incoming sinval messages for unshared catalogs, so we won't realize it
894+
* if the snapshot has been invalidated. Assume it's no good anymore.
895+
*/
896+
InvalidateCatalogSnapshot();
897+
884898
/*
885899
* Recheck pg_database to make sure the target database hasn't gone away.
886900
* If there was a concurrent DROP DATABASE, this ensures we will die

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp