This patch got stuck due to our limited understand of python's dynamic loading
of shared libraries.  I picked it up again ince I have some downtime..

I think that way it's supposed to work, for an OS distribution, is that, at
compile time, whatever library is linked to, its SONAME is used, but also, its
*minor* version is captured by the distribution tools, and creates a package
dependency on library>=minorversion.  That means if you install a new version
of some tool, it might pull in a new version of a library, even if that's a
minor version.  Or it might pull in a totally new library package, since
distributions usually allow simultaneously installing libfoo1 (major=1) and
libfoo2 (major=2) to allow rolling transitions.  The debian tool that handles
that is dpkg-shlibdeps, and I know RHEL has something similar.

I don't see how that can work with pip, though..

It looks like python uses BIND_NOW when it dlopen's a library, like when pygres
does "from _pg import *".  Maybe because it *can't* work with pip.

I was able to make this work as I originally intended like:

$ PYTHONPATH=build/lib.linux-x86_64-3.5 python3 -c "import sys; 
sys.setdlopenflags(1); import pg; print(pg.DB('postgres').query('SELECT 
generate_series(1,9999)').memsize())" 
None

sys says:
|Among other things, this will enable a lazy resolving of symbols when 
importing a module, if called as sys.setdlopenflags(0).

But that didn't work for me.. Also, ctypes say:
|On posix systems, RTLD_NOW is always added, and is not configurable.

In any case, I don't think calling sys modules before importing _pg is a good
solution.  So I think the "solution" is to use a compile time check only, and
assume that it won't be run against an older version.  (And if need be, wait
until the new symbol is common enough that's a nonissue).

Since it's BIND_NOW, it means that any python program linked to pygres will
simply fail at startup in the unlikely even that it was compiled against libpq
for PG12 but runs against an lib older than 12.  It doesn't run for awhile and
then crash when the function is eventually called, which is bad.

On Sat, Oct 05, 2019 at 11:04:35AM -0500, Justin Pryzby wrote:
> On Sat, Oct 05, 2019 at 04:48:40PM +0200, Christoph Zwerschke wrote:
> > >Find attached minimal patch.
> 
> I thought it ought to have compile time check (for headers and link library
> version) and runtime check (for runtime lib version), but, now, even *with* my
> runtime library check, this happens if pygres is run against an earlier 
> version
> of libpq:
> 
> |$ PYTHONPATH=build/lib.linux-x86_64-2.7/ python2.7 -c "import pg; print 
> pg.DB('postgres').query('SELECT generate_series(1,9999)').memsize()"
> |Traceback (most recent call last):
> |  File "<string>", line 1, in <module>
> |  File "pg.py", line 27, in <module>
> |    from _pg import *
> |ImportError: /home/pryzbyj/src/pygres/build/lib.linux-x86_64-2.7/_pg.so: 
> undefined symbol: PQresultMemorySize
> 
> ..so maybe my runtime check is useless, or maybe libpq isn't doing proper
> library versioning?  I've forgotten whatever I once knew about this.
> 
> $ ldd build/lib.linux-x86_64-2.7/_pg.so |grep libpq
>         libpq.so.5 => /usr/lib/x86_64-linux-gnu/libpq.so.5 
> (0x00007fb3f652a000)

On Sat, Oct 05, 2019 at 11:47:40AM -0500, Justin Pryzby wrote:
> On Sat, Oct 05, 2019 at 06:37:36PM +0200, Christoph Zwerschke wrote:
> > >|$ PYTHONPATH=build/lib.linux-x86_64-2.7/ python2.7 -c "import pg; print 
> > >pg.DB('postgres').query('SELECT generate_series(1,9999)').memsize()"
> > >|Traceback (most recent call last):
> > >|  File "<string>", line 1, in <module>
> > >|  File "pg.py", line 27, in <module>
> > >|    from _pg import *
> > >|ImportError: /home/pryzbyj/src/pygres/build/lib.linux-x86_64-2.7/_pg.so: 
> > >undefined symbol: PQresultMemorySize
> > 
> > Are you sure you compiled that with the pgconfig.h of Postgres 12? setup.py
> > is using the includes specified by the pgconfig tool.
> 
> The intended behavior is to hit the runtime version check and then return 
> None.
> To check, I compiled against pg12 and then downgraded libpq to v10.
> 
> Built like:
> PATH=/usr/lib/postgresql/12/bin:$PATH ./setup.py build
> 
> If I hadn't, then it wouldn't have referenced PQresultMemorySize at all, 
> right ?

On Sat, Oct 05, 2019 at 01:53:23PM -0500, Justin Pryzby wrote:
> On Sat, Oct 05, 2019 at 07:47:50PM +0200, Christoph Zwerschke wrote:
> > Am 05.10.2019 um 18:47 schrieb Justin Pryzby:
> > > On Sat, Oct 05, 2019 at 06:37:36PM +0200, Christoph Zwerschke wrote:
> > >> Are you sure you compiled that with the pgconfig.h of Postgres 12?
> > >> setup.py is using the includes specified by the pgconfig tool.
> > > The intended behavior is to hit the runtime version check and then
> > > return None.
> > > To check, I compiled against pg12 and then downgraded libpq to v10.
> > > Built like: PATH=/usr/lib/postgresql/12/bin:$PATH ./setup.py build
> > > If I hadn't, then it wouldn't have referenced PQresultMemorySize at
> > > all, right ?
> > 
> > Ah, yes, you're right. The runtime checks are useless because you can't even
> > load a shared library that doesn't support all the symbols that you are
> > referencing somewhere.
> 
> In general, that's false, due to lazy binding (man ld.so: /LD_BIND_NOW/)
> ...but maybe true for python's c modules ??

This seems to be the case.

On Sun, Oct 06, 2019 at 08:20:10AM -0500, Justin Pryzby wrote:
> Remember, lazy binding is the default on my machine, and my patch doesn't DWIW
> here..  So I don't think adding lazy link options is going to help.  I think
> because of something python is doing (??)

-- 
Justin
_______________________________________________
PyGreSQL mailing list
PyGreSQL@Vex.Net
https://mail.vex.net/mailman/listinfo/pygresql

Reply via email to