[Patch] Check file type before calling AllocateFile() for files out of pg data directory to avoid potential issues (e.g. hang).

From: Paul Guo <pguo(at)pivotal(dot)io>
To: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: [Patch] Check file type before calling AllocateFile() for files out of pg data directory to avoid potential issues (e.g. hang).
Date: 2019-04-24 04:46:15
Message-ID: CAEET0ZHozYa3=_fCQo6o-9hw32Shv9Md6DAZV9di9aXaL9a83A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello, Postgres hackers.

I happened to see a hang issue when running a simple copy query. The root
cause and repro way are quite simple.

mkfifo /tmp/a

run sql:
copy (select generate_series(1, 10)) to '/tmp/a';

It hangs at AllocateFile()->fopen() because that file was previously
created as a fifo file, and it is not ctrl+c cancellable (on Linux).

#0 0x00007f52df1c45a0 in __open_nocancel () at
../sysdeps/unix/syscall-template.S:81
#1 0x00007f52df151f20 in _IO_file_open (is32not64=4, read_write=4,
prot=438, posix_mode=<optimized out>, filename=0x7ffe64199a10
"\360\303[\001", fp=0x1649c40) at fileops.c:229
#2 _IO_new_file_fopen (fp=fp(at)entry=0x1649c40,
filename=filename(at)entry=0x15bc3f0
"/tmp/a", mode=<optimized out>, mode(at)entry=0xaa0bb7 "w",
is32not64=is32not64(at)entry=1) at fileops.c:339
#3 0x00007f52df1465e4 in __fopen_internal (filename=0x15bc3f0 "/tmp/a",
mode=0xaa0bb7 "w", is32=1) at iofopen.c:90
#4 0x00000000007a0e90 in AllocateFile (name=0x15bc3f0 "/tmp/a",
mode=mode(at)entry=0xaa0bb7 "w") at fd.c:2229
#5 0x00000000005c51b4 in BeginCopyTo (pstate=pstate(at)entry=0x15b95f0,
rel=rel(at)entry=0x0, query=<optimized out>, queryRelId=queryRelId(at)entry=0,
filename=<optimized out>, is_program=<optimized out>, attnamelist=0x0,
options=0x0) at copy.c:1919
#6 0x00000000005c8999 in DoCopy (pstate=pstate(at)entry=0x15b95f0,
stmt=stmt(at)entry=0x1596b60, stmt_location=0, stmt_len=48,
processed=processed(at)entry=0x7ffe64199cd8) at copy.c:1078
#7 0x00000000007d717a in standard_ProcessUtility (pstmt=0x1596ea0,
queryString=0x1595dc0 "copy (select generate_series(1, 10)) to '/tmp/a';",
context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x1596f80,
completionTag=0x7ffe64199f90 "") at utility.c:551

This is, in theory, not a 100% bug, but it is probably not unusual to see
conflicts of files between postgresql sqls and other applications on the
same node so I think the fix is needed. I checked all code that calls
AllocateFile() and wrote a simple patch to do sanity check (if the file
exists it must be a regular file) for those files which are probably out of
the postgres data directories which we probably want to ignore. This is
actually not a perfect fix since it is not atomic (check and open), but it
should fix most of the scenarios. To be perfect, we might want to refactor
AllocateFile() to allow atomic check&create using either 'x' in fopen()
or O_EXCL in open(), also it seems that we might not want to create temp
file for AllocateFile() with fixed filenames. This is beyond of this patch
of course.

Thanks.

Attachment Content-Type Size
0001-Make-sure-the-file-is-a-regular-file-if-it-exists-be.patch application/octet-stream 2.6 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2019-04-24 04:49:31 Re: [Patch] Check file type before calling AllocateFile() for files out of pg data directory to avoid potential issues (e.g. hang).
Previous Message Kyotaro HORIGUCHI 2019-04-24 04:23:04 Re: Regression test PANICs with master-standby setup on same machine