MIME-Version: 1.0
From: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Date: Mon, 20 Apr 2026 01:39:51 +0530
Message-ID: 
 <CAJTYsWWg6tFrdMhWs5PkwESTNeeUUsMuY17O4UmPPh771c3stA@mail.gmail.com>
Subject: [BUG] Race in online checksums launcher_exit()
To: pgsql-hackers@postgresql.org
Content-Type: multipart/mixed; boundary="000000000000812653064fd5c4d0"
Archived-At: 
 <https://www.postgresql.org/message-id/CAJTYsWWg6tFrdMhWs5PkwESTNeeUUsMuY17O4UmPPh771c3stA%40mail.gmail.com>
Precedence: bulk

--000000000000812653064fd5c4d0
Content-Type: multipart/alternative; boundary="000000000000812652064fd5c4ce"

--000000000000812652064fd5c4ce
Content-Type: text/plain; charset="UTF-8"

Hi hackers,

While using the pg_enable_data_checksums() feature, I found a likely bug, a
race condition in  datachecksum_state.c's launcher_exit().

When pg_enable_data_checksums() is called twice before the first launcher
starts, two bg workers are registered (the code expects this).  The
redundant launcher exits early, but it's launcher_exit() callback
unconditionally clears the shared launcher_running flag and may call
SetDataChecksumsOff() -- even though it never owned the flag.

This allows a third pg_enable_data_checksums() call to launch another
launcher concurrently with the first (duplicate work, doubled I/O, spurious
warnings).  Worse, if the redundant launcher initialized after the winner
transitioned to inprogress-on, its exit handler calls
SetDataChecksumsOff(), silently aborting the enable operation.  (I have
not triggered the SetDataChecksumsOff part though calling out ad it can be
a likely scenario based on timing of workers)

Reproduced by firing three calls in quick succession:

  psql -c "SELECT pg_enable_data_checksums();" &
  psql -c "SELECT pg_enable_data_checksums();" &
  sleep 0.5
  psql -c "SELECT pg_enable_data_checksums();" &

Log shows two launchers processing databases concurrently:

  [2093292] LOG:  enabling data checksums requested
  [2093293] LOG:  already running, exiting
  [2093299] LOG:  enabling data checksums requested     -- third launcher
admitted
  [2093292] LOG:  processing database "postgres"
  [2093299] LOG:  processing database "postgres"        -- same DB,
concurrently
  [2093299] WARNING:  cannot set data checksums to "on", current state is
not "inprogress-on"

I think the process-local launcher_running flag exists for this purpose and
is already used for the worker-kill block, but the flag-clear and
state-revert blocks do not use it.

The attached patch returns early from launcher_exit() when the local flag
is false. Thoughts?

Regards,
Ayush

--000000000000812652064fd5c4ce
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">Hi hackers,<br><br>While using the pg_enable_data_checksum=
s() feature, I found a likely bug, a race condition in=C2=A0 datachecksum_s=
tate.c&#39;s launcher_exit().<br><br>When pg_enable_data_checksums() is cal=
led twice before the first launcher starts, two bg workers are registered (=
the code expects this).=C2=A0 The redundant launcher exits early, but it=
9;s launcher_exit() callback unconditionally clears the shared launcher_run=
ning flag and may call SetDataChecksumsOff() -- even though it never owned =
the flag. <br><br>This allows a third pg_enable_data_checksums() call to la=
unch another launcher concurrently with the first (duplicate work, doubled =
I/O, spurious warnings).=C2=A0 Worse, if the redundant launcher initialized=
 after the winner transitioned to inprogress-on, its exit handler calls Set=
DataChecksumsOff(), silently aborting the enable operation.=C2=A0

(I have not=C2=A0triggered the SetDataChecksumsOff part though=C2=A0calling=
 out ad it can be a likely scenario based on timing of workers)

<br><br>Reproduced by firing three calls in quick succession:<br><br>=C2=A0=
 psql -c &quot;SELECT pg_enable_data_checksums();&quot; &amp;<br>=C2=A0 psq=
l -c &quot;SELECT pg_enable_data_checksums();&quot; &amp;<br>=C2=A0 sleep 0=
.5<br>=C2=A0 psql -c &quot;SELECT pg_enable_data_checksums();&quot; &amp;<b=
r><br>Log shows two launchers processing databases concurrently:<br><br>=C2=
=A0 [2093292] LOG: =C2=A0enabling data checksums requested<br>=C2=A0 [20932=
93] LOG: =C2=A0already running, exiting<br>=C2=A0 [2093299] LOG: =C2=A0enab=
ling data checksums requested=C2=A0 =C2=A0 =C2=A0-- third launcher admitted=
<br>=C2=A0 [2093292] LOG: =C2=A0processing database &quot;postgres&quot;<br=
>=C2=A0 [2093299] LOG: =C2=A0processing database &quot;postgres&quot;=C2=A0=
 =C2=A0 =C2=A0 =C2=A0 -- same DB, concurrently<br>=C2=A0 [2093299] WARNING:=
 =C2=A0cannot set data checksums to &quot;on&quot;, current state is not &q=
uot;inprogress-on&quot;<br><br>I think the process-local launcher_running f=
lag exists for this purpose and is already used for the worker-kill block, =
but the flag-clear and state-revert blocks do not use it.<br><br>The attach=
ed patch returns early from launcher_exit() when the local flag is false. T=
houghts?<br><br>Regards,<br>Ayush</div>

--000000000000812652064fd5c4ce--

--000000000000812653064fd5c4d0
Content-Type: application/octet-stream;
	name="0001-Fix-race-in-online-checksums-launcher_exit.patch"
Content-Disposition: attachment;
	filename="0001-Fix-race-in-online-checksums-launcher_exit.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mo66yh9l0>
X-Attachment-Id: f_mo66yh9l0

RnJvbSAzMzZkNWI2NzExNTdmOTc0Y2NlZWYxNTM4NWUzOTRjYTI3ZGQ1OGYyIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBBeXVzaCBUaXdhcmkgPGF5dXNodGl3YXJpLnNsZzAxQGdtYWls
LmNvbT4KRGF0ZTogTW9uLCAyMCBBcHIgMjAyNiAwMDo1MjowMCArMDUzMApTdWJqZWN0OiBbQlVH
XSBGaXggcmFjZSBpbiBvbmxpbmUgY2hlY2tzdW1zIGxhdW5jaGVyX2V4aXQoKQoKV2hlbiBwZ19l
bmFibGVfZGF0YV9jaGVja3N1bXMoKSBpcyBjYWxsZWQgdHdpY2UgYmVmb3JlIHRoZSBmaXJzdAps
YXVuY2hlciBzdGFydHMsIHR3byBsYXVuY2hlciBwcm9jZXNzZXMgYXJlIHJlZ2lzdGVyZWQuICBU
aGUgc2Vjb25kCihyZWR1bmRhbnQpIGxhdW5jaGVyIGV4aXRzIGVhcmx5IGFmdGVyIHNlZWluZyBs
YXVuY2hlcl9ydW5uaW5nIGlzCmFscmVhZHkgc2V0LCBidXQgaXRzIGxhdW5jaGVyX2V4aXQoKSBj
YWxsYmFjayB1bmNvbmRpdGlvbmFsbHkgY2xlYXJzCnRoZSBzaGFyZWQgRGF0YUNoZWNrc3VtU3Rh
dGUtPmxhdW5jaGVyX3J1bm5pbmcgZmxhZyBhbmQgbWF5IGNhbGwKU2V0RGF0YUNoZWNrc3Vtc09m
ZigpLiAgVGhpcyBhbGxvd3MgYSB0aGlyZCBsYXVuY2hlciB0byBzdGFydApjb25jdXJyZW50bHkg
d2l0aCB0aGUgZmlyc3QsIGFuZCBjYW4gc2lsZW50bHkgcmV2ZXJ0IHRoZSBjbHVzdGVyCmNoZWNr
c3VtIHN0YXRlIHRvIG9mZiB3aGlsZSB0aGUgZmlyc3QgbGF1bmNoZXIgaXMgc3RpbGwgd29ya2lu
Zy4KCkZpeCBieSByZXR1cm5pbmcgZWFybHkgZnJvbSBsYXVuY2hlcl9leGl0KCkgd2hlbiB0aGUg
cHJvY2Vzcy1sb2NhbApsYXVuY2hlcl9ydW5uaW5nIGZsYWcgaXMgZmFsc2UsIGluZGljYXRpbmcg
dGhpcyBwcm9jZXNzIG5ldmVyIGNsYWltZWQKdGhlIGxhdW5jaGVyIHJvbGUuCi0tLQogc3JjL2Jh
Y2tlbmQvcG9zdG1hc3Rlci9kYXRhY2hlY2tzdW1fc3RhdGUuYyB8IDI1ICsrKysrKysrKysrKyst
LS0tLS0tLQogMSBmaWxlIGNoYW5nZWQsIDE2IGluc2VydGlvbnMoKyksIDkgZGVsZXRpb25zKC0p
CgpkaWZmIC0tZ2l0IGEvc3JjL2JhY2tlbmQvcG9zdG1hc3Rlci9kYXRhY2hlY2tzdW1fc3RhdGUu
YyBiL3NyYy9iYWNrZW5kL3Bvc3RtYXN0ZXIvZGF0YWNoZWNrc3VtX3N0YXRlLmMKaW5kZXggMTg3
OTdhOGVlM2QuLjc2ZjVhYTAwZjJiIDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC9wb3N0bWFzdGVy
L2RhdGFjaGVja3N1bV9zdGF0ZS5jCisrKyBiL3NyYy9iYWNrZW5kL3Bvc3RtYXN0ZXIvZGF0YWNo
ZWNrc3VtX3N0YXRlLmMKQEAgLTg4NywxNyArODg3LDI0IEBAIGxhdW5jaGVyX2V4aXQoaW50IGNv
ZGUsIERhdHVtIGFyZykKIHsKIAlhYm9ydF9yZXF1ZXN0ZWQgPSBmYWxzZTsKIAotCWlmIChsYXVu
Y2hlcl9ydW5uaW5nKQorCS8qCisJICogT25seSBwZXJmb3JtIGNsZWFudXAgaWYgd2UgYWN0dWFs
bHkgY2xhaW1lZCB0aGUgbGF1bmNoZXIgcm9sZSBieQorCSAqIHNldHRpbmcgdGhlIHNoYXJlZCBs
YXVuY2hlcl9ydW5uaW5nIGZsYWcuICBBIHJlZHVuZGFudCBsYXVuY2hlciB0aGF0CisJICogZm91
bmQgYW5vdGhlciBsYXVuY2hlciBhbHJlYWR5IHJ1bm5pbmcgd2lsbCBoYXZlIGV4aXRlZCBlYXJs
eSB3aXRob3V0CisJICogc2V0dGluZyB0aGUgbG9jYWwgbGF1bmNoZXJfcnVubmluZyBmbGFnLCBh
bmQgbXVzdCBub3QgdG91Y2ggdGhlIHNoYXJlZAorCSAqIHN0YXRlIG93bmVkIGJ5IHRoZSBhY3Rp
dmUgbGF1bmNoZXIuCisJICovCisJaWYgKCFsYXVuY2hlcl9ydW5uaW5nKQorCQlyZXR1cm47CisK
KwlMV0xvY2tBY3F1aXJlKERhdGFDaGVja3N1bXNXb3JrZXJMb2NrLCBMV19FWENMVVNJVkUpOwor
CWlmIChEYXRhQ2hlY2tzdW1TdGF0ZS0+d29ya2VyX3BpZCAhPSBJbnZhbGlkUGlkKQogCXsKLQkJ
TFdMb2NrQWNxdWlyZShEYXRhQ2hlY2tzdW1zV29ya2VyTG9jaywgTFdfRVhDTFVTSVZFKTsKLQkJ
aWYgKERhdGFDaGVja3N1bVN0YXRlLT53b3JrZXJfcGlkICE9IEludmFsaWRQaWQpCi0JCXsKLQkJ
CWVyZXBvcnQoTE9HLAotCQkJCQllcnJtc2coImRhdGEgY2hlY2tzdW1zIGxhdW5jaGVyIGV4aXRp
bmcgd2hpbGUgd29ya2VyIGlzIHN0aWxsIHJ1bm5pbmcsIHNpZ25hbGxpbmcgd29ya2VyIikpOwot
CQkJa2lsbChEYXRhQ2hlY2tzdW1TdGF0ZS0+d29ya2VyX3BpZCwgU0lHVEVSTSk7Ci0JCX0KLQkJ
TFdMb2NrUmVsZWFzZShEYXRhQ2hlY2tzdW1zV29ya2VyTG9jayk7CisJCWVyZXBvcnQoTE9HLAor
CQkJCWVycm1zZygiZGF0YSBjaGVja3N1bXMgbGF1bmNoZXIgZXhpdGluZyB3aGlsZSB3b3JrZXIg
aXMgc3RpbGwgcnVubmluZywgc2lnbmFsbGluZyB3b3JrZXIiKSk7CisJCWtpbGwoRGF0YUNoZWNr
c3VtU3RhdGUtPndvcmtlcl9waWQsIFNJR1RFUk0pOwogCX0KKwlMV0xvY2tSZWxlYXNlKERhdGFD
aGVja3N1bXNXb3JrZXJMb2NrKTsKIAogCS8qCiAJICogSWYgdGhlIGxhdW5jaGVyIGlzIGV4aXRp
bmcgYmVmb3JlIGRhdGEgY2hlY2tzdW1zIGFyZSBlbmFibGVkIHRoZW4gc2V0Ci0tIAoyLjM0LjEK
Cg==
--000000000000812653064fd5c4d0--