Change pgcrypto to use the new ResourceOwner mechanism.

This is a nice example of how extensions can now use ResourceOwners to
track extension-specific resource kinds

Reviewed-by: Peter Eisentraut, Andres Freund
Discussion: https://www.postgresql.org/message-id/d746cead-a1ef-7efe-fb47-933311e876a3%40iki.fi

diff --git a/contrib/pgcrypto/openssl.c b/contrib/pgcrypto/openssl.c
index cf315517e0cba6a18653a547659d0ae390c7c0b0..4a913bd04f763cadc6cd3897a1a4e42c4bcd1981 100644 (file)
--- a/contrib/pgcrypto/openssl.c
+++ b/contrib/pgcrypto/openssl.c
@@ -50,9 +50,8 @@
  */
 
 /*
- * To make sure we don't leak OpenSSL handles on abort, we keep OSSLDigest
- * objects in a linked list, allocated in TopMemoryContext. We use the
- * ResourceOwner mechanism to free them on abort.
+ * To make sure we don't leak OpenSSL handles, we use the ResourceOwner
+ * mechanism to free them on abort.
  */
 typedef struct OSSLDigest
 {
@@ -60,54 +59,39 @@ typedef struct OSSLDigest
        EVP_MD_CTX *ctx;
 
        ResourceOwner owner;
-       struct OSSLDigest *next;
-       struct OSSLDigest *prev;
 } OSSLDigest;
 
-static OSSLDigest *open_digests = NULL;
-static bool digest_resowner_callback_registered = false;
+/* ResourceOwner callbacks to hold OpenSSL digest handles */
+static void ResOwnerReleaseOSSLDigest(Datum res);
 
-static void
-free_openssl_digest(OSSLDigest *digest)
+static const ResourceOwnerDesc ossldigest_resowner_desc =
 {
-       EVP_MD_CTX_destroy(digest->ctx);
-       if (digest->prev)
-               digest->prev->next = digest->next;
-       else
-               open_digests = digest->next;
-       if (digest->next)
-               digest->next->prev = digest->prev;
-       pfree(digest);
+       .name = "pgcrypto OpenSSL digest handle",
+       .release_phase = RESOURCE_RELEASE_BEFORE_LOCKS,
+       .release_priority = RELEASE_PRIO_FIRST,
+       .ReleaseResource = ResOwnerReleaseOSSLDigest,
+       .DebugPrint = NULL,                     /* default message is fine */
+};
+
+/* Convenience wrappers over ResourceOwnerRemember/Forget */
+static inline void
+ResourceOwnerRememberOSSLDigest(ResourceOwner owner, OSSLDigest *digest)
+{
+       ResourceOwnerRemember(owner, PointerGetDatum(digest), &ossldigest_resowner_desc);
+}
+static inline void
+ResourceOwnerForgetOSSLDigest(ResourceOwner owner, OSSLDigest *digest)
+{
+       ResourceOwnerForget(owner, PointerGetDatum(digest), &ossldigest_resowner_desc);
 }
 
-/*
- * Close any open OpenSSL handles on abort.
- */
 static void
-digest_free_callback(ResourceReleasePhase phase,
-                                        bool isCommit,
-                                        bool isTopLevel,
-                                        void *arg)
+free_openssl_digest(OSSLDigest *digest)
 {
-       OSSLDigest *curr;
-       OSSLDigest *next;
-
-       if (phase != RESOURCE_RELEASE_AFTER_LOCKS)
-               return;
-
-       next = open_digests;
-       while (next)
-       {
-               curr = next;
-               next = curr->next;
-
-               if (curr->owner == CurrentResourceOwner)
-               {
-                       if (isCommit)
-                               elog(WARNING, "pgcrypto digest reference leak: digest %p still referenced", curr);
-                       free_openssl_digest(curr);
-               }
-       }
+       EVP_MD_CTX_destroy(digest->ctx);
+       if (digest->owner != NULL)
+               ResourceOwnerForgetOSSLDigest(digest->owner, digest);
+       pfree(digest);
 }
 
 static unsigned
@@ -188,16 +172,12 @@ px_find_digest(const char *name, PX_MD **res)
                OpenSSL_add_all_algorithms();
        }
 
-       if (!digest_resowner_callback_registered)
-       {
-               RegisterResourceReleaseCallback(digest_free_callback, NULL);
-               digest_resowner_callback_registered = true;
-       }
-
        md = EVP_get_digestbyname(name);
        if (md == NULL)
                return PXE_NO_HASH;
 
+       ResourceOwnerEnlarge(CurrentResourceOwner);
+
        /*
         * Create an OSSLDigest object, an OpenSSL MD object, and a PX_MD object.
         * The order is crucial, to make sure we don't leak anything on
@@ -221,9 +201,7 @@ px_find_digest(const char *name, PX_MD **res)
        digest->algo = md;
        digest->ctx = ctx;
        digest->owner = CurrentResourceOwner;
-       digest->next = open_digests;
-       digest->prev = NULL;
-       open_digests = digest;
+       ResourceOwnerRememberOSSLDigest(digest->owner, digest);
 
        /* The PX_MD object is allocated in the current memory context. */
        h = palloc(sizeof(*h));
@@ -239,6 +217,17 @@ px_find_digest(const char *name, PX_MD **res)
        return 0;
 }
 
+/* ResourceOwner callbacks for OSSLDigest */
+
+static void
+ResOwnerReleaseOSSLDigest(Datum res)
+{
+       OSSLDigest *digest = (OSSLDigest *) DatumGetPointer(res);
+
+       digest->owner = NULL;
+       free_openssl_digest(digest);
+}
+
 /*
  * Ciphers
  *
@@ -266,9 +255,8 @@ struct ossl_cipher
  * OSSLCipher contains the state for using a cipher. A separate OSSLCipher
  * object is allocated in each px_find_cipher() call.
  *
- * To make sure we don't leak OpenSSL handles on abort, we keep OSSLCipher
- * objects in a linked list, allocated in TopMemoryContext. We use the
- * ResourceOwner mechanism to free them on abort.
+ * To make sure we don't leak OpenSSL handles, we use the ResourceOwner
+ * mechanism to free them on abort.
  */
 typedef struct OSSLCipher
 {
@@ -281,54 +269,39 @@ typedef struct OSSLCipher
        const struct ossl_cipher *ciph;
 
        ResourceOwner owner;
-       struct OSSLCipher *next;
-       struct OSSLCipher *prev;
 } OSSLCipher;
 
-static OSSLCipher *open_ciphers = NULL;
-static bool cipher_resowner_callback_registered = false;
+/* ResourceOwner callbacks to hold OpenSSL cipher state */
+static void ResOwnerReleaseOSSLCipher(Datum res);
 
-static void
-free_openssl_cipher(OSSLCipher *od)
+static const ResourceOwnerDesc osslcipher_resowner_desc =
 {
-       EVP_CIPHER_CTX_free(od->evp_ctx);
-       if (od->prev)
-               od->prev->next = od->next;
-       else
-               open_ciphers = od->next;
-       if (od->next)
-               od->next->prev = od->prev;
-       pfree(od);
+       .name = "pgcrypto OpenSSL cipher handle",
+       .release_phase = RESOURCE_RELEASE_BEFORE_LOCKS,
+       .release_priority = RELEASE_PRIO_FIRST,
+       .ReleaseResource = ResOwnerReleaseOSSLCipher,
+       .DebugPrint = NULL,                     /* default message is fine */
+};
+
+/* Convenience wrappers over ResourceOwnerRemember/Forget */
+static inline void
+ResourceOwnerRememberOSSLCipher(ResourceOwner owner, OSSLCipher *od)
+{
+       ResourceOwnerRemember(owner, PointerGetDatum(od), &osslcipher_resowner_desc);
+}
+static inline void
+ResourceOwnerForgetOSSLCipher(ResourceOwner owner, OSSLCipher *od)
+{
+       ResourceOwnerForget(owner, PointerGetDatum(od), &osslcipher_resowner_desc);
 }
 
-/*
- * Close any open OpenSSL cipher handles on abort.
- */
 static void
-cipher_free_callback(ResourceReleasePhase phase,
-                                        bool isCommit,
-                                        bool isTopLevel,
-                                        void *arg)
+free_openssl_cipher(OSSLCipher *od)
 {
-       OSSLCipher *curr;
-       OSSLCipher *next;
-
-       if (phase != RESOURCE_RELEASE_AFTER_LOCKS)
-               return;
-
-       next = open_ciphers;
-       while (next)
-       {
-               curr = next;
-               next = curr->next;
-
-               if (curr->owner == CurrentResourceOwner)
-               {
-                       if (isCommit)
-                               elog(WARNING, "pgcrypto cipher reference leak: cipher %p still referenced", curr);
-                       free_openssl_cipher(curr);
-               }
-       }
+       EVP_CIPHER_CTX_free(od->evp_ctx);
+       if (od->owner != NULL)
+               ResourceOwnerForgetOSSLCipher(od->owner, od);
+       pfree(od);
 }
 
 /* Common routines for all algorithms */
@@ -782,11 +755,7 @@ px_find_cipher(const char *name, PX_Cipher **res)
        if (i->name == NULL)
                return PXE_NO_CIPHER;
 
-       if (!cipher_resowner_callback_registered)
-       {
-               RegisterResourceReleaseCallback(cipher_free_callback, NULL);
-               cipher_resowner_callback_registered = true;
-       }
+       ResourceOwnerEnlarge(CurrentResourceOwner);
 
        /*
         * Create an OSSLCipher object, an EVP_CIPHER_CTX object and a PX_Cipher.
@@ -806,9 +775,7 @@ px_find_cipher(const char *name, PX_Cipher **res)
 
        od->evp_ctx = ctx;
        od->owner = CurrentResourceOwner;
-       od->next = open_ciphers;
-       od->prev = NULL;
-       open_ciphers = od;
+       ResourceOwnerRememberOSSLCipher(od->owner, od);
 
        if (i->ciph->cipher_func)
                od->evp_ciph = i->ciph->cipher_func();
@@ -827,3 +794,11 @@ px_find_cipher(const char *name, PX_Cipher **res)
        *res = c;
        return 0;
 }
+
+/* ResourceOwner callbacks for OSSLCipher */
+
+static void
+ResOwnerReleaseOSSLCipher(Datum res)
+{
+       free_openssl_cipher((OSSLCipher *) DatumGetPointer(res));
+}