Обсуждение: [BUG] lo_open() makes a warning/falls to an assertion

Is it possible to reproduce on your system?

I got the following result at the latest CVS HEAD independently from
the feature of SE-PostgreSQL. I could not get the result a week ago,
but could not find where cause the regression.

-- (1) make a dummy file.
[kaigai@saba base]$ dd if=/dev/zero of=/tmp/hoge bs=1024 count=80
80+0 records in
80+0 records out
81920 bytes (82 kB) copied, 0.000767843 s, 107 MB/s

-- (2) import it.
[kaigai@saba base]$ psql postgres
psql (8.4devel)
Type "help" for help.

postgres=# SELECT lo_import('/tmp/hoge');lo_import
-----------    24576
(1 row)

-- (3) tries to open it with writable mode under transaction block.
postgres=# BEGIN;
BEGIN
postgres=# SELECT lo_open(24576, x'40000'::int);
WARNING:  Snapshot reference leak: Snapshot 0x96b8488 still referencedlo_open
---------      0
(1 row)

postgres=# ABORT;
ROLLBACK

-- (4) tries to open it with writable mode outside of transaction block.
postgres=# SELECT lo_open(24576, x'40000'::int);
WARNING:  Snapshot reference leak: Snapshot 0x96b8488 still referenced
server closed the connection unexpectedly       This probably means the server terminated abnormally       before or
whileprocessing the request.
 
The connection to the server was lost. Attempting reset: Succeeded.

In this case, it is killed by an assertion macro.
----------------
[kaigai@saba base]$ gdb -f /usr/local/pgsql/bin/postgres -c /tmp/core
GNU gdb Fedora (6.8-29.fc10)
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i386-redhat-linux-gnu"...

warning: Can't read pathname for load map: Input/output error.
Reading symbols from /lib/libdl.so.2...done.
Loaded symbols for /lib/libdl.so.2
Reading symbols from /lib/libm.so.6...done.
Loaded symbols for /lib/libm.so.6
Reading symbols from /lib/libc.so.6...done.
Loaded symbols for /lib/libc.so.6
Reading symbols from /lib/ld-linux.so.2...done.
Loaded symbols for /lib/ld-linux.so.2
Reading symbols from /lib/libnss_files.so.2...done.
Loaded symbols for /lib/libnss_files.so.2
Core was generated by `postgres: kaigai postgres [loc'.
Program terminated with signal 6, Aborted.
[New process 30031]
#0  0x00110416 in __kernel_vsyscall ()
Missing separate debuginfos, use: debuginfo-install glibc-2.9-2.i686
(gdb) bt
#0  0x00110416 in __kernel_vsyscall ()
#1  0x0086f450 in raise () from /lib/libc.so.6
#2  0x00870e18 in abort () from /lib/libc.so.6
#3  0x083093be in ExceptionalCondition (conditionName=0x846094c "!(RegisteredSnapshots > 0)",   errorType=0x833e818
"FailedAssertion",fileName=0x84608f5 "snapmgr.c", lineNumber=387)   at assert.c:57
 
#4  0x083361bc in UnregisterSnapshot (snapshot=0x96b8488) at snapmgr.c:387
#5  0x08240469 in inv_close (obj_desc=0x96c0f14) at inv_api.c:274
#6  0x081b9223 in AtEOXact_LargeObject (isCommit=1 '\001') at be-fsstubs.c:511
#7  0x080c3a8c in CommitTransaction () at xact.c:1608
#8  0x080c4b55 in CommitTransactionCommand () at xact.c:2331
#9  0x0824e9e9 in finish_xact_command () at postgres.c:2330
#10 0x0824fa35 in exec_simple_query (query_string=0x9688644 "SELECT lo_open(24576, x'40000'::int);")   at
postgres.c:1025
#11 0x082510f4 in PostgresMain (argc=4, argv=0x9626e18, username=0x9626de8 "kaigai")   at postgres.c:3573
#12 0x0821c20f in BackendRun () at postmaster.c:3258
#13 BackendStartup () at postmaster.c:2872
#14 ServerLoop () at postmaster.c:1283
#15 0x0821d190 in PostmasterMain (argc=1, argv=0x96249e8) at postmaster.c:1031
#16 0x081c20b6 in main (argc=1, argv=0x96249e8) at main.c:188
(gdb)
----------------

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

KaiGai Kohei wrote:

> -- (3) tries to open it with writable mode under transaction block.
> postgres=# BEGIN;
> BEGIN
> postgres=# SELECT lo_open(24576, x'40000'::int);
> WARNING:  Snapshot reference leak: Snapshot 0x96b8488 still referenced

Interesting.  This is due to this patch
http://git.postgresql.org/?p=postgresql.git;a=commit;h=52d70aa5d0cdcdf9ab925fb46024b4b91760ac43

I'm investigating now.  Thanks for the report.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera wrote:
> KaiGai Kohei wrote:
> 
> > -- (3) tries to open it with writable mode under transaction block.
> > postgres=# BEGIN;
> > BEGIN
> > postgres=# SELECT lo_open(24576, x'40000'::int);
> > WARNING:  Snapshot reference leak: Snapshot 0x96b8488 still referenced

> I'm investigating now.  Thanks for the report.

I think the solution is to have each large object have its own
ResourceOwner, and store the snapshot in it.  Otherwise the snapshot is
left in the calling query's resowner, which is not good.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera wrote:

> I think the solution is to have each large object have its own
> ResourceOwner, and store the snapshot in it.  Otherwise the snapshot is
> left in the calling query's resowner, which is not good.

Turns out to be overkill.  This patch solves the problem, by using the
transaction's resowner.  I'm now going to check if we need something
similar elsewhere.

--
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Here's a better patch.

--
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera <alvherre@commandprompt.com> writes:
> I think the solution is to have each large object have its own
> ResourceOwner, and store the snapshot in it.  Otherwise the snapshot is
> left in the calling query's resowner, which is not good.

That's not gonna scale to transactions that touch lots of large objects
(think pg_dump).  Also it seems like it would be arbitrarily altering
the past behavior of LOs.  Why don't they just use the calling query's
snapshot (which I think is what the historical behavior is, though I'm
too lazy to check right now)?
        regards, tom lane

Tom Lane wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
> > I think the solution is to have each large object have its own
> > ResourceOwner, and store the snapshot in it.  Otherwise the snapshot is
> > left in the calling query's resowner, which is not good.
> 
> That's not gonna scale to transactions that touch lots of large objects
> (think pg_dump).  Also it seems like it would be arbitrarily altering
> the past behavior of LOs.  Why don't they just use the calling query's
> snapshot (which I think is what the historical behavior is, though I'm
> too lazy to check right now)?

Well, that's precisely the problem -- they cannot use the query snapshot
(which is what the current buggy code is doing), because the query goes
away just after calling lo_open, but the snapshot needs to survive until
after lo_close is called (or until end of xact).  So the last patch I
sent deals with the problem by recording the snapshot in the
transaction's resowner instead, by momentarily setting it as current.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support