Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash due to txns in emit_log_hook_callback #2987

Merged
merged 1 commit into from Mar 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions test/src/bgw/log.c
Expand Up @@ -61,6 +61,11 @@ bgw_log_insert(char *msg)

static emit_log_hook_type prev_emit_log_hook = NULL;

/*
* NOTE: using transactions in emit_log_hook functions is not recommended.
* However we rely on this current functionality for our test verifications,
* so have to live with it for now.
*/
static void
emit_log_hook_callback(ErrorData *edata)
{
Expand All @@ -72,6 +77,10 @@ emit_log_hook_callback(ErrorData *edata)
if (MyProc == NULL)
return;

/* We are only interested in elevel LOG and above. */
if (edata->elevel < LOG)
return;

/*
* Block signals so we don't lose messages generated during signal
* processing if they occur while we are saving this log message (since
Expand Down Expand Up @@ -107,6 +116,10 @@ emit_log_hook_callback(ErrorData *edata)
}
PG_CATCH();
{
/* If there was an error, rollback what was done before the error */
if (IsTransactionState())
AbortCurrentTransaction();
nikkhils marked this conversation as resolved.
Show resolved Hide resolved

/*
* Reinstall the hook because we are out of the main body of the
* function.
Expand Down
20 changes: 20 additions & 0 deletions tsl/src/remote/connection.c
Expand Up @@ -1776,6 +1776,13 @@ remote_connections_cleanup(SubTransactionId subtxid, bool isabort)
static void
remote_connection_xact_end(XactEvent event, void *unused_arg)
{
/*
* We are deep down in CommitTransaction code path. We do not want our
* emit_log_hook_callback to interfere since it uses its own transaction
*/
emit_log_hook_type prev_emit_log_hook = emit_log_hook;
emit_log_hook = NULL;

switch (event)
{
case XACT_EVENT_ABORT:
Expand All @@ -1789,12 +1796,22 @@ remote_connection_xact_end(XactEvent event, void *unused_arg)
default:
break;
}

/* re-enable the emit_log_hook */
emit_log_hook = prev_emit_log_hook;
}

static void
remote_connection_subxact_end(SubXactEvent event, SubTransactionId subtxid,
SubTransactionId parent_subtxid, void *unused_arg)
{
/*
* We are deep down in CommitTransaction code path. We do not want our
* emit_log_hook_callback to interfere since it uses its own transaction
*/
emit_log_hook_type prev_emit_log_hook = emit_log_hook;
emit_log_hook = NULL;

switch (event)
{
case SUBXACT_EVENT_ABORT_SUB:
Expand All @@ -1806,6 +1823,9 @@ remote_connection_subxact_end(SubXactEvent event, SubTransactionId subtxid,
default:
break;
}

/* re-enable the emit_log_hook */
emit_log_hook = prev_emit_log_hook;
}

bool
Expand Down