| Lists: | pgsql-hackers |
|---|
| From: | Andres Freund <andres(at)2ndquadrant(dot)com> |
|---|---|
| To: | pgsql-hackers(at)postgresql(dot)org |
| Subject: | buffile.c resource owner breakage on segment extension |
| Date: | 2013-11-01 18:31:07 |
| Message-ID: | 20131101183107.GA3567@awork2.anarazel.de |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Hi,
The attached testcase demonstrates that it currently is possible that
buffile.c segments get created belonging to the wrong resource owner
leading to WARNINGs ala "temporary file leak: File %d still referenced",
ERRORs like "write failed", asserts and segfaults.
The problem is that while BufFileCreateTemp() callers like tuplestore
take care to use proper resource owners for it, they don't during
BufFileWrite()->BufFileDumpBuffer()->extendBufFile(). The last in that
chain creates a new tempfile which then gets owned by
CurrentResourceOwner. Which, like in the testcase, might a
subtransaction's one.
While not particularly nice, given the API, it seems best for buffile.c
to remember the resource owner used for the original segment and
temporarily set that during the extension.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
| Attachment | Content-Type | Size |
|---|---|---|
| fix-buffile-extension-resowner.patch | text/x-patch | 1.5 KB |
| leak_file.sql | text/plain | 738 bytes |
| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
|---|---|
| To: | Andres Freund <andres(at)2ndquadrant(dot)com> |
| Cc: | pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: buffile.c resource owner breakage on segment extension |
| Date: | 2013-11-01 19:28:54 |
| Message-ID: | 19266.1383334134@sss.pgh.pa.us |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> While not particularly nice, given the API, it seems best for buffile.c
> to remember the resource owner used for the original segment and
> temporarily set that during the extension.
Hm, yeah, that seems right. It's just like repalloc keeping the memory
chunk in its original context. The comments here are a bit inadequate...
regards, tom lane
| From: | Andres Freund <andres(at)2ndquadrant(dot)com> |
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: buffile.c resource owner breakage on segment extension |
| Date: | 2013-11-04 09:44:11 |
| Message-ID: | 20131104094410.GG3567@awork2.anarazel.de |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Lists: | pgsql-hackers |
On 2013-11-01 15:28:54 -0400, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > While not particularly nice, given the API, it seems best for buffile.c
> > to remember the resource owner used for the original segment and
> > temporarily set that during the extension.
>
> Hm, yeah, that seems right. It's just like repalloc keeping the memory
> chunk in its original context. The comments here are a bit inadequate...
Thanks for committing and sorry for the need to freshen up the
comments. I don't think I had ever opened buffile.c before and thus
wasn't sure if there isn't a better fix, so I didn't want to spend too
much time on it before somebody agreed it is the right fix.
Also, I was actually just trying to recover some data from a corrupted
database and this stopped me from it ;)
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services