Skip to content

Commit 23cbe57

Browse files
authored
fix: remove pthread_mutex that causes self-deadlock on VLE queries (#2433)
The pthread_mutex in manage_GRAPH_global_contexts() causes permanent self-deadlock when ereport(ERROR) triggers siglongjmp while the mutex is held, skipping pthread_mutex_unlock(). Any subsequent VLE query on the same backend connection hangs forever in pthread_mutex_lock() with __owner == own PID. The mutex was introduced in PR #1881 (fix for issue #1878) but is unnecessary: it protects a process-local static variable in PostgreSQL's single-threaded backend model where no concurrent access exists. The actual fix for #1878 was the Assert-to-runtime-check conversion and strndup defensive copy, which remain untouched.
1 parent 639c8d5 commit 23cbe57

1 file changed

Lines changed: 13 additions & 50 deletions

File tree

src/backend/utils/adt/age_global_graph.c

Lines changed: 13 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@
4545
#include "catalog/ag_label.h"
4646
#include "utils/ag_cache.h"
4747

48-
#include <pthread.h>
4948

5049
/* defines */
5150
#define VERTEX_HTAB_NAME "Vertex to edge lists " /* added a space at end for */
@@ -152,18 +151,8 @@ typedef struct GRAPH_global_context
152151
struct GRAPH_global_context *next; /* next graph */
153152
} GRAPH_global_context;
154153

155-
/* container for GRAPH_global_context and its mutex */
156-
typedef struct GRAPH_global_context_container
157-
{
158-
/* head of the list */
159-
GRAPH_global_context *contexts;
160-
161-
/* mutex to protect the list */
162-
pthread_mutex_t mutex_lock;
163-
} GRAPH_global_context_container;
164-
165154
/* global variable to hold the per process GRAPH global contexts */
166-
static GRAPH_global_context_container global_graph_contexts_container = {0};
155+
static GRAPH_global_context *global_graph_contexts = NULL;
167156

168157
/*
169158
* VertexEdgeArray helpers — flat-array adjacency container used by
@@ -1054,12 +1043,10 @@ GRAPH_global_context *manage_GRAPH_global_contexts(char *graph_name,
10541043
* 5) One or more other contexts do exist but, one or more are invalid.
10551044
*/
10561045

1057-
/* lock the global contexts list */
1058-
pthread_mutex_lock(&global_graph_contexts_container.mutex_lock);
10591046

10601047
/* free the invalidated GRAPH global contexts first */
10611048
prev_ggctx = NULL;
1062-
curr_ggctx = global_graph_contexts_container.contexts;
1049+
curr_ggctx = global_graph_contexts;
10631050
while (curr_ggctx != NULL)
10641051
{
10651052
GRAPH_global_context *next_ggctx = curr_ggctx->next;
@@ -1076,7 +1063,7 @@ GRAPH_global_context *manage_GRAPH_global_contexts(char *graph_name,
10761063
*/
10771064
if (prev_ggctx == NULL)
10781065
{
1079-
global_graph_contexts_container.contexts = next_ggctx;
1066+
global_graph_contexts = next_ggctx;
10801067
}
10811068
else
10821069
{
@@ -1089,8 +1076,6 @@ GRAPH_global_context *manage_GRAPH_global_contexts(char *graph_name,
10891076
/* if it wasn't successfull, there was a missing vertex entry */
10901077
if (!success)
10911078
{
1092-
/* unlock the mutex so we don't get a deadlock */
1093-
pthread_mutex_unlock(&global_graph_contexts_container.mutex_lock);
10941079

10951080
ereport(ERROR, (errcode(ERRCODE_DATA_EXCEPTION),
10961081
errmsg("missing vertex or edge entry during free")));
@@ -1106,16 +1091,14 @@ GRAPH_global_context *manage_GRAPH_global_contexts(char *graph_name,
11061091
}
11071092

11081093
/* find our graph's context. if it exists, we are done */
1109-
curr_ggctx = global_graph_contexts_container.contexts;
1094+
curr_ggctx = global_graph_contexts;
11101095
while (curr_ggctx != NULL)
11111096
{
11121097
if (curr_ggctx->graph_oid == graph_oid)
11131098
{
11141099
/* switch our context back */
11151100
MemoryContextSwitchTo(oldctx);
11161101

1117-
/* we are done unlock the global contexts list */
1118-
pthread_mutex_unlock(&global_graph_contexts_container.mutex_lock);
11191102

11201103
return curr_ggctx;
11211104
}
@@ -1125,17 +1108,17 @@ GRAPH_global_context *manage_GRAPH_global_contexts(char *graph_name,
11251108
/* otherwise, we need to create one and possibly attach it */
11261109
new_ggctx = palloc0(sizeof(GRAPH_global_context));
11271110

1128-
if (global_graph_contexts_container.contexts != NULL)
1111+
if (global_graph_contexts != NULL)
11291112
{
1130-
new_ggctx->next = global_graph_contexts_container.contexts;
1113+
new_ggctx->next = global_graph_contexts;
11311114
}
11321115
else
11331116
{
11341117
new_ggctx->next = NULL;
11351118
}
11361119

11371120
/* set the global context variable */
1138-
global_graph_contexts_container.contexts = new_ggctx;
1121+
global_graph_contexts = new_ggctx;
11391122

11401123
/* set the graph name and oid */
11411124
new_ggctx->graph_name = pstrdup(graph_name);
@@ -1157,8 +1140,6 @@ GRAPH_global_context *manage_GRAPH_global_contexts(char *graph_name,
11571140
load_GRAPH_global_hashtables(new_ggctx);
11581141
freeze_GRAPH_global_hashtables(new_ggctx);
11591142

1160-
/* unlock the global contexts list */
1161-
pthread_mutex_unlock(&global_graph_contexts_container.mutex_lock);
11621143

11631144
/* switch back to the previous memory context */
11641145
MemoryContextSwitchTo(oldctx);
@@ -1170,18 +1151,16 @@ GRAPH_global_context *manage_GRAPH_global_contexts(char *graph_name,
11701151
* Helper function to delete all of the global graph contexts used by the
11711152
* process. When done the global global_graph_contexts will be NULL.
11721153
*
1173-
* NOTE: Function uses a MUTEX global_graph_contexts_mutex
1154+
*
11741155
*/
11751156
static bool delete_GRAPH_global_contexts(void)
11761157
{
11771158
GRAPH_global_context *curr_ggctx = NULL;
11781159
bool retval = false;
11791160

1180-
/* lock contexts list */
1181-
pthread_mutex_lock(&global_graph_contexts_container.mutex_lock);
11821161

11831162
/* get the first context, if any */
1184-
curr_ggctx = global_graph_contexts_container.contexts;
1163+
curr_ggctx = global_graph_contexts;
11851164

11861165
/* free all GRAPH global contexts */
11871166
while (curr_ggctx != NULL)
@@ -1195,8 +1174,6 @@ static bool delete_GRAPH_global_contexts(void)
11951174
/* if it wasn't successfull, there was a missing vertex entry */
11961175
if (!success)
11971176
{
1198-
/* unlock the mutex so we don't get a deadlock */
1199-
pthread_mutex_unlock(&global_graph_contexts_container.mutex_lock);
12001177

12011178
ereport(ERROR, (errcode(ERRCODE_DATA_EXCEPTION),
12021179
errmsg("missing vertex or edge entry during free")));
@@ -1209,10 +1186,8 @@ static bool delete_GRAPH_global_contexts(void)
12091186
}
12101187

12111188
/* reset the head of the contexts to NULL */
1212-
global_graph_contexts_container.contexts = NULL;
1189+
global_graph_contexts = NULL;
12131190

1214-
/* unlock the global contexts list */
1215-
pthread_mutex_unlock(&global_graph_contexts_container.mutex_lock);
12161191

12171192
return retval;
12181193
}
@@ -1235,11 +1210,9 @@ static bool delete_specific_GRAPH_global_contexts(char *graph_name)
12351210
/* get the graph oid */
12361211
graph_oid = get_graph_oid(graph_name);
12371212

1238-
/* lock the global contexts list */
1239-
pthread_mutex_lock(&global_graph_contexts_container.mutex_lock);
12401213

12411214
/* get the first context, if any */
1242-
curr_ggctx = global_graph_contexts_container.contexts;
1215+
curr_ggctx = global_graph_contexts;
12431216

12441217
/* find the specified GRAPH global context */
12451218
while (curr_ggctx != NULL)
@@ -1256,7 +1229,7 @@ static bool delete_specific_GRAPH_global_contexts(char *graph_name)
12561229
*/
12571230
if (prev_ggctx == NULL)
12581231
{
1259-
global_graph_contexts_container.contexts = next_ggctx;
1232+
global_graph_contexts = next_ggctx;
12601233
}
12611234
else
12621235
{
@@ -1266,8 +1239,6 @@ static bool delete_specific_GRAPH_global_contexts(char *graph_name)
12661239
/* free the current graph context */
12671240
success = free_specific_GRAPH_global_context(curr_ggctx);
12681241

1269-
/* unlock the global contexts list */
1270-
pthread_mutex_unlock(&global_graph_contexts_container.mutex_lock);
12711242

12721243
/* if it wasn't successfull, there was a missing vertex entry */
12731244
if (!success)
@@ -1285,8 +1256,6 @@ static bool delete_specific_GRAPH_global_contexts(char *graph_name)
12851256
curr_ggctx = next_ggctx;
12861257
}
12871258

1288-
/* unlock the global contexts list */
1289-
pthread_mutex_unlock(&global_graph_contexts_container.mutex_lock);
12901259

12911260
/* we didn't find it, return false */
12921261
return false;
@@ -1347,19 +1316,15 @@ GRAPH_global_context *find_GRAPH_global_context(Oid graph_oid)
13471316
{
13481317
GRAPH_global_context *ggctx = NULL;
13491318

1350-
/* lock the global contexts lists */
1351-
pthread_mutex_lock(&global_graph_contexts_container.mutex_lock);
13521319

13531320
/* get the root */
1354-
ggctx = global_graph_contexts_container.contexts;
1321+
ggctx = global_graph_contexts;
13551322

13561323
while(ggctx != NULL)
13571324
{
13581325
/* if we found it return it */
13591326
if (ggctx->graph_oid == graph_oid)
13601327
{
1361-
/* unlock the global contexts lists */
1362-
pthread_mutex_unlock(&global_graph_contexts_container.mutex_lock);
13631328

13641329
return ggctx;
13651330
}
@@ -1368,8 +1333,6 @@ GRAPH_global_context *find_GRAPH_global_context(Oid graph_oid)
13681333
ggctx = ggctx->next;
13691334
}
13701335

1371-
/* unlock the global contexts lists */
1372-
pthread_mutex_unlock(&global_graph_contexts_container.mutex_lock);
13731336

13741337
/* we did not find it so return NULL */
13751338
return NULL;

0 commit comments

Comments
 (0)