Re: Data race in interfaces/libpq/fe-exec.c - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Data race in interfaces/libpq/fe-exec.c
Date
Msg-id 22989.1580402784@sss.pgh.pa.us
Whole thread Raw
In response to Data race in interfaces/libpq/fe-exec.c  (Mark Charsley <mcharsley@google.com>)
Responses Re: Data race in interfaces/libpq/fe-exec.c  (Mark Charsley <mcharsley@google.com>)
List pgsql-hackers
Mark Charsley <mcharsley@google.com> writes:
> This line
>
https://github.com/postgres/postgres/blob/30012a04a6c8127397a8ab71e160d9c7e7fbe874/src/interfaces/libpq/fe-exec.c#L1073
> triggers data race errors when run under ThreadSanitizer (*)

> As far as I can tell, the static variable in question is a hack to allow a
> couple of deprecated functions that are already unsafe to use
> (PQescapeString and PQescapeBytea) to be fractionally less unsafe to use.

Yup.

> Would there be any interest in a patch changing the type of
> static_client_coding
> and static_std_strings
>
<https://github.com/postgres/postgres/blob/30012a04a6c8127397a8ab71e160d9c7e7fbe874/src/interfaces/libpq/fe-exec.c#L49>
> to
> some atomic equivalent, so the data race goes away?

I don't see that making those be some other datatype would improve anything
usefully.  (1) On just about every platform known to man, int and bool are
going to be atomic anyway.  (2) The *actual* hazards here, as opposed to
theoretical ones, are that you're using more than one connection with
different settings for these values, whereupon it's not clear whether
those deprecated functions will see the appropriate settings when they're
used.  A different data type won't help that.

In short: this warning you're getting from ThreadSanitizer is entirely
off-point, so contorting the code to suppress it seems useless.

            regards, tom lane



pgsql-hackers by date:

Previous
From: Fujii Masao
Date:
Subject: Re: table partitioning and access privileges
Next
From: Fujii Masao
Date:
Subject: Re: recovery_target_action=pause with confusing hint