Skip to content

Commit

Permalink
[#12884, #15938] YSQL: Fix TSAN issues caused by quickdie()
Browse files Browse the repository at this point in the history
Summary:
quickdie() is the signal handler invoked to handle SIGQUIT. This is the
immediate shutdown mode, expected to be invoked when something is really
wrong, such as corrupted shared memory.

This being the case, it is not safe to invoke
YBOnPostgresBackendShutdown, which internally invokes PgApi destructor
from quickdie. This also causes TSAN failures because there could be
other concurrent threads in process of accessing the PgApi object.

Secondly as noted in the comment in quickdie(), it is not safe to call
ereport() from quickdie as it is not async-signal-safe. It has been
added by PG authors anyway as a best effort to provide some information before shutting
down. Although acceptable, this again causes TSAN failures. Therefore
this patch silences the TSAN failures by not invoking the ereport() in
TSAN build.

Test Plan: Manually tested by running tests enabled with TSAN.

Reviewers: dmitry

Reviewed By: dmitry

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D23045
  • Loading branch information
deeps1991 committed Feb 21, 2023
1 parent 57a7d98 commit c5f2231
Showing 1 changed file with 4 additions and 6 deletions.
10 changes: 4 additions & 6 deletions src/postgres/src/backend/tcop/postgres.c
Original file line number Diff line number Diff line change
Expand Up @@ -2703,6 +2703,8 @@ quickdie(SIGNAL_ARGS)
* Ideally this should be ereport(FATAL), but then we'd not get control
* back...
*/

#ifndef THREAD_SANITIZER
ereport(WARNING,
(errcode(ERRCODE_CRASH_SHUTDOWN),
errmsg("terminating connection because of crash of another server process"),
Expand All @@ -2712,10 +2714,7 @@ quickdie(SIGNAL_ARGS)
" shared memory."),
errhint("In a moment you should be able to reconnect to the"
" database and repeat your command.")));

if (IsYugaByteEnabled()) {
YBOnPostgresBackendShutdown();
}
#endif

/*
* We DO NOT want to run proc_exit() or atexit() callbacks -- we're here
Expand Down Expand Up @@ -2750,9 +2749,8 @@ die(SIGNAL_ARGS)
ProcDiePending = true;
}

if (IsYugaByteEnabled()) {
if (IsYugaByteEnabled())
YBCInterruptPgGate();
}

/* If we're still here, waken anything waiting on the process latch */
SetLatch(MyLatch);
Expand Down

0 comments on commit c5f2231

Please sign in to comment.