pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Christoph Berg <myon@debian.org>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: pgsql: Remove pqsignal() from libpq's official exports list.
Date: Tue, 08 Oct 2019 16:25:05 -0400
Message-ID: <9333.1570566305@sss.pgh.pa.us> (raw)
In-Reply-To: <20191004095630.GA29119@msg.df7cb.de>
References: <E1g5vmT-0003K1-6S@gemulon.postgresql.org>
	<20191004095630.GA29119@msg.df7cb.de>

Christoph Berg <myon@debian.org> writes:
> Re: Tom Lane 2018-09-28 <E1g5vmT-0003K1-6S@gemulon.postgresql.org>
>> Remove pqsignal() from libpq's official exports list.

> This is starting to hurt in several places:
> 04 11:41 <magnush> mha@xindi:~$ psql
> 04 11:41 <magnush> /usr/lib/postgresql/9.2/bin/psql: symbol lookup error:
>                    /usr/lib/postgresql/9.2/bin/psql: undefined symbol: pqsignal

I poked into this a little.  Reviewing the commit history, pqsignal()
was a part of libpq (so far as frontend code is concerned) up until
9.3, when commit da5aeccf6 moved it into libpgport.  Since then we've
expected client programs to get it from libpgport not libpq, and indeed
they do so AFAICT --- I can reproduce the described failure with 9.2
and below psql linking to current libpq.so, but not with 9.3 and up.

libpq itself indeed has no need for pqsignal at all, if
--enable-thread-safety is set, which it usually is these days.

I also notice that we've made at least one nontrivial semantics change
to pqsignal: commit 873ab9721 made it use SA_RESTART for SIGALRM
handlers, which it did not do before 9.3.  So really, none of the
post-9.2 versions of libpq have faithfully duplicated what an older
client would expect from pqsignal.  This isn't at all academic, because
I see that pgbench uses pqsignal(SIGALRM,...), and so does pg_test_fsync.
Now, I don't see any indication that we've adjusted either of those
programs for the different behavior, so maybe it's fine.  But we've been
treating this function as strictly internal, and so I'm not pleased with
the idea of putting it back into the exported symbol list.

I'm especially not pleased with doing so to support pre-9.3 client
programs.  Those have been below our support horizon for some time;
notably, they (presumably) haven't been patched for CVE-2018-1058.
Why are you still shipping them in current OS releases?

> pg_repack linked against libpq5 11 breaks with libpq5 12:

This probably means it needs to be linked with libpgport
not only libpq.

Having said all that, if we conclude we can't break compatibility
with this legacy code quite yet, I'd be inclined to put a
separate, clearly-marked-as-legacy-code version of pqsignal()
back into libpq, using the pre-9.3 SA_RESTART semantics.
But I'd like some pretty well-defined sunset time for that,
because it'd just be trouble waiting to happen.  When are
you going to remove 9.2 psql?

			regards, tom lane





view thread (9+ messages)  latest in thread

Message-ID: <9333.1570566305@sss.pgh.pa.us>
Permalink:  ../9333.1570566305@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/9333.1570566305@sss.pgh.pa.us

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-committers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, myon@debian.org, pgsql-hackers@postgresql.org
  Subject: Re: pgsql: Remove pqsignal() from libpq's official exports list.
  In-Reply-To: <9333.1570566305@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox