Re: [PATCH] Use DatumGetUInt32() for dsm_attach() in test_shm_mq_main() - Mailing list pgsql-hackers

From Jianghua Yang
Subject Re: [PATCH] Use DatumGetUInt32() for dsm_attach() in test_shm_mq_main()
Date
Msg-id CAAZLFmS4g1TLzLxQpVhOMiKn3fJ=VGtCJcbwh_bu-B=u4AMjHQ@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] Use DatumGetUInt32() for dsm_attach() in test_shm_mq_main()  (Nathan Bossart <nathandbossart@gmail.com>)
Responses Re: [PATCH] Use DatumGetUInt32() for dsm_attach() in test_shm_mq_main()
List pgsql-hackers


Hi,

Just to follow up — in our production system (pg_cron extension), we’ve encountered real issues caused by passing a `Datum` to `dsm_attach()` using `DatumGetInt32()` instead of `DatumGetUInt32()`.

Here's a sample of the errors observed in our logs:

ERROR: unable to map dynamic shared memory segment
WARNING: one or more background workers failed to start


These errors trace back to failures in `dsm_attach()`, where the segment handle value was incorrectly interpreted due to sign extension from `int32`. 
The patch proposed earlier resolves this issue by correctly using `DatumGetUInt32()`.


Thanks,  
Jianghua yang



Nathan Bossart <nathandbossart@gmail.com> 于2025年6月26日周四 13:31写道:
On Thu, Jun 26, 2025 at 12:51:10PM -0700, Jianghua Yang wrote:
> The argument passed to `dsm_attach()` is expected to be a `uint32`, but the
> code currently uses `DatumGetInt32()` to extract it from the `Datum`
> argument. This can lead to incorrect behavior when the high bit is set, as
> 'unable to map dynamic shared memory segment'.

I'm not sure this actually causes any problems in practice because
dsm_attach() treats its argument as unsigned.  In any case, I've never seen
this test fail like that, and presumably the high bit is sometimes set
because the handle is generated with a PRNG.

Nevertheless, I see no point in using the wrong macro.  I'll plan on
committing/back-patching this shortly.

--
nathan

pgsql-hackers by date:

Previous
From: Nathan Bossart
Date:
Subject: Re: [PATCH] Use DatumGetUInt32() for dsm_attach() in test_shm_mq_main()
Next
From: Nathan Bossart
Date:
Subject: Re: [PATCH] Add tests for binaryheap.c