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

Commit247263d

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 parentb6a3444 commit247263d

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
@@ -819,28 +819,20 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
819819
strcpy(out_dbname,dbname);
820820
}
821821

822-
/* Now we can mark our PGPROC entry with the database ID */
823-
/* (We assume this is an atomic store so no lock is needed) */
824-
MyProc->databaseId=MyDatabaseId;
825-
826-
/*
827-
* We established a catalog snapshot while reading pg_authid and/or
828-
* pg_database; but until we have set up MyDatabaseId, we won't react to
829-
* incoming sinval messages for unshared catalogs, so we won't realize it
830-
* if the snapshot has been invalidated. Assume it's no good anymore.
831-
*/
832-
InvalidateCatalogSnapshot();
833-
834822
/*
835823
* Now, take a writer's lock on the database we are trying to connect to.
836824
* If there is a concurrently running DROP DATABASE on that database, this
837825
* will block us until it finishes (and has committed its update of
838826
* pg_database).
839827
*
840828
* Note that the lock is not held long, only until the end of this startup
841-
* transaction. This is OK since we are already advertising our use of
842-
* the database in the PGPROC array; anyone trying a DROP DATABASE after
843-
* this point will see us there.
829+
* transaction. This is OK since we will advertise our use of the
830+
* database in the ProcArray before dropping the lock (in fact, that's the
831+
* next thing to do). Anyone trying a DROP DATABASE after this point will
832+
* see us in the array once they have the lock. Ordering is important for
833+
* this because we don't want to advertise ourselves as being in this
834+
* database until we have the lock; otherwise we create what amounts to a
835+
* deadlock with CountOtherDBBackends().
844836
*
845837
* Note: use of RowExclusiveLock here is reasonable because we envision
846838
* our session as being a concurrent writer of the database. If we had a
@@ -852,6 +844,28 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
852844
LockSharedObject(DatabaseRelationId,MyDatabaseId,0,
853845
RowExclusiveLock);
854846

847+
/*
848+
* Now we can mark our PGPROC entry with the database ID.
849+
*
850+
* We assume this is an atomic store so no lock is needed; though actually
851+
* things would work fine even if it weren't atomic. Anyone searching the
852+
* ProcArray for this database's ID should hold the database lock, so they
853+
* would not be executing concurrently with this store. A process looking
854+
* for another database's ID could in theory see a chance match if it read
855+
* a partially-updated databaseId value; but as long as all such searches
856+
* wait and retry, as in CountOtherDBBackends(), they will certainly see
857+
* the correct value on their next try.
858+
*/
859+
MyProc->databaseId=MyDatabaseId;
860+
861+
/*
862+
* We established a catalog snapshot while reading pg_authid and/or
863+
* pg_database; but until we have set up MyDatabaseId, we won't react to
864+
* incoming sinval messages for unshared catalogs, so we won't realize it
865+
* if the snapshot has been invalidated. Assume it's no good anymore.
866+
*/
867+
InvalidateCatalogSnapshot();
868+
855869
/*
856870
* Recheck pg_database to make sure the target database hasn't gone away.
857871
* If there was a concurrent DROP DATABASE, this ensures we will die

0 commit comments

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