errno clobbering in reorderbuffer

Started by Alvaro Herreraover 9 years ago7 messageshackers
Jump to latest
#1Alvaro Herrera
alvherre@2ndquadrant.com

While researching a customer issue with BDR I noticed that one ereport()
call happens after clobbering errno, leading to the wrong strerror being
reported. This patch fixes it by saving before calling
CloseTransientFile and restoring afterwards.

I also threw in a missing errcode I noticed while looking for similar
problems in the same file.

This is to backpatch to 9.4.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

reorderbuffer-errno.patchtext/x-diff; charset=us-asciiDownload+5-1
#2Andres Freund
andres@anarazel.de
In reply to: Alvaro Herrera (#1)
Re: errno clobbering in reorderbuffer

On 2016-08-18 19:06:02 -0300, Alvaro Herrera wrote:

While researching a customer issue with BDR I noticed that one ereport()
call happens after clobbering errno, leading to the wrong strerror being
reported. This patch fixes it by saving before calling
CloseTransientFile and restoring afterwards.

I also threw in a missing errcode I noticed while looking for similar
problems in the same file.

This is to backpatch to 9.4.

Makes sense - you're doing that or shall I?

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#1)
Re: errno clobbering in reorderbuffer

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

While researching a customer issue with BDR I noticed that one ereport()
call happens after clobbering errno, leading to the wrong strerror being
reported. This patch fixes it by saving before calling
CloseTransientFile and restoring afterwards.

I also threw in a missing errcode I noticed while looking for similar
problems in the same file.

Looks sane to me.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Andres Freund
andres@anarazel.de
In reply to: Alvaro Herrera (#1)
Re: errno clobbering in reorderbuffer

Hi,

On 2016-08-18 19:06:02 -0300, Alvaro Herrera wrote:

if (write(fd, rb->outbuf, ondisk->size) != ondisk->size)
{
+ int save_errno = errno;
+
CloseTransientFile(fd);
+ errno = save_errno;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not write to data file for XID %u: %m",

Independent of this specific case I kinda wish we had a better way to
deal with exactly this pattern. I even wonder whether having a close
variant not clobbering errno might be worthwhile.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Andres Freund (#2)
Re: errno clobbering in reorderbuffer

Andres Freund wrote:

On 2016-08-18 19:06:02 -0300, Alvaro Herrera wrote:

While researching a customer issue with BDR I noticed that one ereport()
call happens after clobbering errno, leading to the wrong strerror being
reported. This patch fixes it by saving before calling
CloseTransientFile and restoring afterwards.

I also threw in a missing errcode I noticed while looking for similar
problems in the same file.

This is to backpatch to 9.4.

Makes sense - you're doing that or shall I?

I am, if you don't mind.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Andres Freund
andres@anarazel.de
In reply to: Alvaro Herrera (#5)
Re: errno clobbering in reorderbuffer

On August 18, 2016 7:21:03 PM PDT, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

Andres Freund wrote:

On 2016-08-18 19:06:02 -0300, Alvaro Herrera wrote:

While researching a customer issue with BDR I noticed that one

ereport()

call happens after clobbering errno, leading to the wrong strerror

being

reported. This patch fixes it by saving before calling
CloseTransientFile and restoring afterwards.

I also threw in a missing errcode I noticed while looking for

similar

problems in the same file.

This is to backpatch to 9.4.

Makes sense - you're doing that or shall I?

I am, if you don't mind.

Not at all, thanks.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Andres Freund (#6)
Re: errno clobbering in reorderbuffer

Andres Freund wrote:

Not at all, thanks.

Done, thanks both for the look-over.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers