pgsql: Properly unregister OpenSSL callbacks when libpq is done with

Log Message:
-----------
Properly unregister OpenSSL callbacks when libpq is done with
it's connection. This is required for applications that unload
the libpq library (such as PHP) in which case we'd otherwise
have pointers to these functions when they no longer exist.

This needs a bit more testing before we can consider a backpatch,
so not doing that yet.

In passing, remove unused functions in backend/libpq.

Bruce Momjian and Magnus Hagander, per report and analysis
by Russell Smith.

Modified Files:
--------------
pgsql/src/backend/libpq:
be-secure.c (r1.86 -> r1.87)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/libpq/be-secure.c?r1=1.86&r2=1.87)
pgsql/src/interfaces/libpq:
fe-secure.c (r1.110 -> r1.111)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/fe-secure.c?r1=1.110&r2=1.111)

Magnus Hagander wrote:
> Log Message:
> -----------
> Properly unregister OpenSSL callbacks when libpq is done with
> it's connection. This is required for applications that unload
> the libpq library (such as PHP) in which case we'd otherwise
> have pointers to these functions when they no longer exist.

Breaks the build with --enable-thread-safety and --with-openssl because
of this typo.

Kris Jurka

Kris Jurka wrote:
> Magnus Hagander wrote:
> > Log Message:
> > -----------
> > Properly unregister OpenSSL callbacks when libpq is done with
> > it's connection. This is required for applications that unload
> > the libpq library (such as PHP) in which case we'd otherwise
> > have pointers to these functions when they no longer exist.
>
> Breaks the build with --enable-thread-safety and --with-openssl because
> of this typo.

Thanks, applied.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> Kris Jurka wrote:
>> Magnus Hagander wrote:
>>> Log Message:
>>> -----------
>>> Properly unregister OpenSSL callbacks when libpq is done with
>>> it's connection. This is required for applications that unload
>>> the libpq library (such as PHP) in which case we'd otherwise
>>> have pointers to these functions when they no longer exist.
>> Breaks the build with --enable-thread-safety and --with-openssl because
>> of this typo.
>
> Thanks, applied.

That fix is wrong.

The comment clearly says the code *shouldn't* free the lockarray, so the
proper fix is to remove those two lines.

I have applied a patch that does this.

//Magnus

[ still catching up on back email ]

mha(at)postgresql(dot)org (Magnus Hagander) writes:
> Properly unregister OpenSSL callbacks when libpq is done with
> it's connection. This is required for applications that unload
> the libpq library (such as PHP) in which case we'd otherwise
> have pointers to these functions when they no longer exist.

I find it fairly disturbing that this patch was committed with a bug
that ensured it wouldn't even compile for the case of SSL and
THREAD_SAFETY both enabled. Which one would think would have been
the primary case to test. Would anyone like to reassure us why this
patch shouldn't just be reverted in toto until it's actually been
tested a bit?

regards, tom lane

Tom Lane wrote:
> [ still catching up on back email ]
>
> mha(at)postgresql(dot)org (Magnus Hagander) writes:
>> Properly unregister OpenSSL callbacks when libpq is done with
>> it's connection. This is required for applications that unload
>> the libpq library (such as PHP) in which case we'd otherwise
>> have pointers to these functions when they no longer exist.
>
> I find it fairly disturbing that this patch was committed with a bug
> that ensured it wouldn't even compile for the case of SSL and
> THREAD_SAFETY both enabled. Which one would think would have been
> the primary case to test. Would anyone like to reassure us why this
> patch shouldn't just be reverted in toto until it's actually been
> tested a bit?

The patch itself was tested. I applied the wrong version of the patch to
the main tree when I moved it from my testing tree to the one where I
apply it from :-( (which was configured without thread-safety)

That part was also included in the part of the patch that was tested by
the PHP guy who originally reported the problem.

//Magnus