On Fri, 2021-01-29 at 13:57 +0100, Daniel Gustafsson wrote: > > On 21 Jan 2021, at 06:21, Michael Paquier <mich...@paquier.xyz> wrote: > > I really need to study more the choide of the options chosen for > > NSS_InitContext()... But based on the docs I can read on the matter I > > think that saving nsscontext in pg_cryptohash_ctx is right for each > > cryptohash built. > > It's a safe but slow option, NSS wasn't really made for running a single > crypto > operation. Since we are opening a context which isn't backed by an NSS > database we could have a static context, which indeed speeds up processing a > lot. The problem with that is that there is no good callsite for closing the > context as the backend is closing down. Since you are kneedeep in the > cryptohash code, do you have any thoughts on this? I've included 0008 which > implements this, with a commented out dummy stub for cleaning up. > > Making nss_context static in cryptohash_nss.c is > appealing but there is no good option for closing it there. Any thoughts on > how to handle global contexts like this?
I'm completely new to this code, so take my thoughts with a grain of salt... I think the bad news is that the static approach will need support for ENABLE_THREAD_SAFETY. (It looks like the NSS implementation of pgtls_close() needs some thread support too?) The good(?) news is that I don't understand why OpenSSL's implementation of cryptohash doesn't _also_ need the thread-safety code. (Shouldn't we need to call CRYPTO_set_locking_callback() et al before using any of its cryptohash implementation?) So maybe we can implement the same global setup/teardown API for OpenSSL too and not have to one-off it for NSS... --Jacob
