pgq/triggers: Fix crash in case invalidate happens from signal handler.

Currently code assumed the invalidate happens only from per-query checks,
but this seems not to be the case.

Fix it by moving reset code out from invalidate callback.

Also old code seemed to leak htab per full reset, because it assumed
to be located under cache context, but init code did not assign it there.

Thanks to Andrew Dunstan for the report.

diff --git a/sql/pgq/triggers/common.c b/sql/pgq/triggers/common.c
index a4c4d2f871074608f7a543bf9438cf1c538cc33f..07e1abd331e0c6e4e696521c55985b9121890e4c 100644 (file)
--- a/sql/pgq/triggers/common.c
+++ b/sql/pgq/triggers/common.c
@@ -42,6 +42,7 @@ PG_MODULE_MAGIC;
  * primary key info
  */
 
+static bool tbl_cache_invalid;
 static MemoryContext tbl_cache_ctx;
 static HTAB *tbl_cache_map;
 
@@ -168,6 +169,17 @@ init_module(void)
 {
        static int callback_init = 0;
 
+       /* do full reset if requested */
+       if (tbl_cache_invalid) {
+               if (tbl_cache_map)
+                       hash_destroy(tbl_cache_map);
+               if (tbl_cache_ctx)
+                       MemoryContextDelete(tbl_cache_ctx);
+               tbl_cache_map = NULL;
+               tbl_cache_ctx = NULL;
+               tbl_cache_invalid = false;
+       }
+
        /* htab can be occasinally dropped */
        if (tbl_cache_ctx)
                return;
@@ -186,25 +198,12 @@ init_module(void)
        }
 }
 
-static void
-full_reset(void)
-{
-       if (tbl_cache_ctx) {
-               /* needed only if backend has HASH_STATISTICS set */
-               /* hash_destroy(tbl_cache_map); */
-               MemoryContextDelete(tbl_cache_ctx);
-               tbl_cache_map = NULL;
-               tbl_cache_ctx = NULL;
-       }
-}
-
 /*
  * Fill table information in hash table.
  */
-static struct PgqTableInfo *
-fill_tbl_info(Relation rel)
+static void
+fill_tbl_info(Relation rel, struct PgqTableInfo *info)
 {
-       struct PgqTableInfo *info;
        StringInfo pkeys;
        Datum values[1];
        const char *name = pgq_find_table_name(rel);
@@ -213,19 +212,15 @@ fill_tbl_info(Relation rel)
        bool isnull;
        int res, i, attno;
 
+       /* allow reset ASAP, but ignore it in this call */
+       info->invalid = false;
+
        /* load pkeys */
        values[0] = ObjectIdGetDatum(rel->rd_id);
        res = SPI_execute_plan(pkey_plan, values, NULL, false, 0);
        if (res != SPI_OK_SELECT)
                elog(ERROR, "pkey_plan exec failed");
 
-       /*
-        * SPI_execute_plan may launch reset callback, thus we need
-        * to load the final position after calling it.
-        */
-       init_module();
-       info = hash_search(tbl_cache_map, &rel->rd_id, HASH_ENTER, NULL);
-
        /*
         * Fill info
         */
@@ -247,8 +242,6 @@ fill_tbl_info(Relation rel)
                appendStringInfoString(pkeys, name);
        }
        info->pkey_list = MemoryContextStrdup(tbl_cache_ctx, pkeys->data);
-
-       return info;
 }
 
 static void
@@ -259,17 +252,19 @@ free_info(struct PgqTableInfo *info)
        pfree((void *)info->pkey_list);
 }
 
+/*
+ * the callback can be launched any time from signal callback,
+ * only minimal tagging can be done here.
+ */
 static void relcache_reset_cb(Datum arg, Oid relid)
 {
        if (relid == InvalidOid) {
-               full_reset();
-       } else if (tbl_cache_map) {
+               tbl_cache_invalid = true;
+       } else if (tbl_cache_map && !tbl_cache_invalid) {
                struct PgqTableInfo *entry;
                entry = hash_search(tbl_cache_map, &relid, HASH_FIND, NULL);
-               if (entry) {
-                       free_info(entry);
-                       hash_search(tbl_cache_map, &relid, HASH_REMOVE, NULL);
-               }
+               if (entry)
+                       entry->invalid = true;
        }
 }
 
@@ -280,12 +275,16 @@ struct PgqTableInfo *
 pgq_find_table_info(Relation rel)
 {
        struct PgqTableInfo *entry;
+       bool found = false;
 
        init_module();
 
-       entry = hash_search(tbl_cache_map, &rel->rd_id, HASH_FIND, NULL);
-       if (!entry)
-               entry = fill_tbl_info(rel);
+       entry = hash_search(tbl_cache_map, &rel->rd_id, HASH_ENTER, &found);
+       if (!found || entry->invalid) {
+               if (found)
+                       free_info(entry);
+               fill_tbl_info(rel, entry);
+       }
 
        return entry;
 }

diff --git a/sql/pgq/triggers/common.h b/sql/pgq/triggers/common.h
index 5be441b18f080509a8398f3f21a953f8ffddae08..50fe834bdf95edf39892d403ae413360dc0f32a7 100644 (file)
--- a/sql/pgq/triggers/common.h
+++ b/sql/pgq/triggers/common.h
@@ -36,6 +36,7 @@ struct PgqTableInfo {
        const char *pkey_list;  /* pk column name list */
        int *pkey_attno;        /* pk column positions */
        char *table_name;       /* schema-quelified table name */
+       bool invalid;           /* set if the info was invalidated */
 };
 
 /* common.c */