Thank you for the explanation! I looked into what OpenSSL does when freeing the underlying OSSLCipher resources, and as you say, it is more than just a free. There is sometimes aditional values to free, and they go through and overwrite the data in a special way regardless. So I see why the TopMemoryContext approach is needed either way.
My particular interest was in using the BIGNUM implementation from OpenSSL within my project, but the BIGNUM frees also do more than just freeing. So it seems there is no way around the TopMemoryContext in my case either.
I could imagine that there may be some effect on the particular character of memory fragmentation that comes with their malloc/free vs palloc/pfree, but that's definitely outside of my personal purposes and outside of my expertise to evaluate even at a high level!
So thank you again for helping me understand this! Happy to have a much clear understanding of the TopMemoryContext approach!
Evan
On Thu, Feb 1, 2024 at 5:01 PM Tom Lane <tgl@xxxxxxxxxxxxx> wrote:
Evan Czaplicki <evancz@xxxxxxxxx> writes:
> I noticed that OpenSSL has a CRYPTO_set_mem_functions
> <https://www.openssl.org/docs/man3.2/man3/CRYPTO_set_mem_functions.html>
> function:
>> If no allocations have been done, it is possible to “swap out” the default
>> implementations for OPENSSL_malloc(), OPENSSL_realloc() and OPENSSL_free()
>> and replace them with alternate versions.
> But a different technique is used in contrib/pgcrypto/openssl.c
>> 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.
> Would it be desirable to do this? If not, why is the TopMemoryContext
> approach a better option? I do not understand the code quite well enough to
> evaluate the tradeoffs myself yet!
Seems to me that these address different purposes. If we put in a
CRYPTO_set_mem_functions layer, I doubt that we'd have any good idea
of which allocations are used for what. So we could not replace what
pgcrypto is doing with a simple MemoryContextReset (even if we cared
to assume that freeing an OSSLCipher involves only free() operations
and no other resources). I think the only real win we'd get from
such a layer is that OpenSSL's allocations would be better exposed
for accounting purposes, eg the pg_backend_memory_contexts view.
That's not negligible, but I don't find it a compelling reason to
do the work, either.
regards, tom lane
