Use thread-specific storage for computing a snapshot.

We mustn't use a global variable since concurrent GTM threads might try to
compute a snapshot at the same time and may overwrite the information before a
thread can send the complete snapshot to the client. Chi Gao
<[email protected]> reported that this can cause infinite wait on the client
side because the client expects N bytes of data, but only receives (N - x)
bytes and it keeps waiting for remaining x bytes which the GTM never sends.
While we don't have a report, it's obvious that it can also go wrong in the
other direction.

We fix this by using a thread-specific storage which ensures that the snapshot
cannot be changed while it's being sent to the client.

Report, analysis and fix by Chi Gao <[email protected]>. Some minor
editorialisation by me.

Backpatched to XL9_5_STABLE

diff --git a/src/gtm/main/gtm_snap.c b/src/gtm/main/gtm_snap.c
index f4ab3a127b58476891f021b9d43ac8d08000945c..5a538985dfb481917ce9dcc98ca1587b95087e19 100644 (file)
--- a/src/gtm/main/gtm_snap.c
+++ b/src/gtm/main/gtm_snap.c
@@ -23,7 +23,6 @@
 #include "gtm/libpq-int.h"
 #include "gtm/pqformat.h"
 
-static GTM_SnapshotData localSnapshot;
 /*
  * Get snapshot for the given transactions. If this is the first call in the
  * transaction, a fresh snapshot is taken and returned back. For a serializable
@@ -91,10 +90,16 @@ GTM_GetTransactionSnapshot(GTM_TransactionHandle handle[], int txn_count, int *s
 
        /*
         * If no valid transaction exists in the array, we record the snapshot in a
-        * local strucure and still send it out to the caller
+        * thread-specific structure. This allows us to avoid repeated
+        * allocation/freeing of the structure.
+        *
+        * Note that we must use a thread-specific variable and not a global
+        * variable because a concurrent thread might compute a new snapshot and
+        * overwrite the snapshot information while we are still sending this copy
+        * to the client. Using a thread-specific storage avoids that problem.
         */
        if (snapshot == NULL)
-               snapshot = &localSnapshot;
+               snapshot = &GetMyThreadInfo->thr_snapshot;
 
        Assert(snapshot != NULL);
 

diff --git a/src/include/gtm/gtm.h b/src/include/gtm/gtm.h
index 8dfa77b1397fb49b6c12ef33f0801feab5b70019..37d31f6ba41b24c4ca54b3732370c1f657748c42 100644 (file)
--- a/src/include/gtm/gtm.h
+++ b/src/include/gtm/gtm.h
@@ -57,7 +57,8 @@ typedef struct GTM_ThreadInfo
        uint32                          thr_client_id;          /* unique client identifier */
 
        GTM_RWLock                      thr_lock;
-       gtm_List                                *thr_cached_txninfo;
+       gtm_List                        *thr_cached_txninfo;
+       GTM_SnapshotData    thr_snapshot;
 } GTM_ThreadInfo;
 
 typedef struct GTM_Threads