Horton

Thread

Commits

  1. Use correct DatumGet*() function in test_shm_mq_main().

  1. [PATCH] Use DatumGetUInt32() for dsm_attach() in test_shm_mq_main()

    Jianghua Yang <yjhjstz@gmail.com> — 2025-06-26T19:51:10Z

    Hi,

The attached patch fixes a minor type mismatch in `test_shm_mq_main()`.

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'.

This patch changes it to use `DatumGetUInt32()` to match the expected type
and ensure correctness.


Thanks,
Jianghua Yang

  2. Re: [PATCH] Use DatumGetUInt32() for dsm_attach() in test_shm_mq_main()

    Nathan Bossart <nathandbossart@gmail.com> — 2025-06-26T20:30:57Z

    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

  3. Re: [PATCH] Use DatumGetUInt32() for dsm_attach() in test_shm_mq_main()

    Jianghua Yang <yjhjstz@gmail.com> — 2025-06-26T20:46:10Z

    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
>

  4. Re: [PATCH] Use DatumGetUInt32() for dsm_attach() in test_shm_mq_main()

    Nathan Bossart <nathandbossart@gmail.com> — 2025-06-26T21:34:00Z

    On Thu, Jun 26, 2025 at 01:46:10PM -0700, Jianghua Yang wrote:
> 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`.

I think there might be something else going on.  I added a debug log in
test_shm_mq, and it looks like it regularly uses handles with the high bit
set.  I also wrote a test program and consulted the C standard, which seem
to confirm that passing a signed integer to a function with an unsigned
parameter leaves the high bit set.

-- 
nathan

  5. Re: [PATCH] Use DatumGetUInt32() for dsm_attach() in test_shm_mq_main()

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-06-27T12:03:12Z

    Nathan Bossart <nathandbossart@gmail.com> writes:
> On Thu, Jun 26, 2025 at 01:46:10PM -0700, Jianghua Yang wrote:
>> These errors trace back to failures in `dsm_attach()`, where the
>> segment handle value was incorrectly interpreted due to sign extension
>> from `int32`.

> I think there might be something else going on.

I agree with Nathan: the patch you proposed is purely cosmetic.
I don't object to it, but you need to dig deeper because this
will not resolve any actual behavioral problem.

			regards, tom lane

  6. Re: [PATCH] Use DatumGetUInt32() for dsm_attach() in test_shm_mq_main()

    Nathan Bossart <nathandbossart@gmail.com> — 2025-06-27T18:44:48Z

    Committed.

-- 
nathan

  7. Re: [PATCH] Use DatumGetUInt32() for dsm_attach() in test_shm_mq_main()

    Jianghua Yang <yjhjstz@gmail.com> — 2025-06-27T19:01:14Z

    Hi Nathan,

Thank you for submitting this patch.

Best regards,
Jianghua


Nathan Bossart <nathandbossart@gmail.com> 于2025年6月27日周五 11:44写道：

> Committed.
>
> --
> nathan
>