Обсуждение: HASH_FIXED_SIZE flag gets lost when attaching to existing hash table

Hi hackers,

Recently, while working with hash tables in dynahash.c, I noticed 
something weird.
When a hash table is already created in shared memory, and the another 
process
calls hash_create attempting to attach to it, it seems like the 
HASH_FIXED_SIZE
flag gets lost.

For example, if you start the server compiled with the EXEC_BACKEND 
option, and a
hash table with a fixed size is created at the beginning in shared 
memory, any
other process started by the postmaster then tries to initialize its 
hash table
pointer by calling hash_create with HASH_ATTACH flag. But the table 
structure it
points to has 'isfixed' flag set to false, even though the original 
table was
created with a HASH_FIXED_SIZE provided.

This could lead to the situation where, when the table's capacity limit 
is reached
(which was specified when the table was created), the process will 
silently occupy
more of the shared memory (up to a certain point?). Then, another insert 
could
trigger an out of shared memory error.

Any thoughts?


Regards,
Aidar Imamov

Aidar Imamov <a.imamov@postgrespro.ru> writes:
> Recently, while working with hash tables in dynahash.c, I noticed 
> something weird.
> When a hash table is already created in shared memory, and the another 
> process
> calls hash_create attempting to attach to it, it seems like the 
> HASH_FIXED_SIZE
> flag gets lost.

Yeah, you are right.  This seems to be an oversight in 7c797e719
which introduced that flag.  It only affects predicate-lock tables
because we don't use HASH_FIXED_SIZE anywhere else, and it'd only
matter in EXEC_BACKEND builds, so it's not that surprising that
nobody noticed.  But we ought to fix it going forward.

I don't really like your solution though.  ISTM the intent of the
code is that if the shared hashtable already exists, we adhere to the
properties it has, we don't rely on the current caller to specify the
exact same values.  So relying on the caller to get HASH_FIXED_SIZE
correct here seems like the wrong thing.  I think we ought to add
an isfixed flag to the shared hashtable header and copy that.
(IOW, isfixed ought to act more like keysize/ssize/sshift, and should
perhaps be grouped with them.)

            regards, tom lane

On 2025-07-24 20:24, Tom Lane wrote:
> Aidar Imamov <a.imamov@postgrespro.ru> writes:
>> Recently, while working with hash tables in dynahash.c, I noticed
>> something weird.
>> When a hash table is already created in shared memory, and the another
>> process
>> calls hash_create attempting to attach to it, it seems like the
>> HASH_FIXED_SIZE
>> flag gets lost.
> 
> Yeah, you are right.  This seems to be an oversight in 7c797e719
> which introduced that flag.  It only affects predicate-lock tables
> because we don't use HASH_FIXED_SIZE anywhere else, and it'd only
> matter in EXEC_BACKEND builds, so it's not that surprising that
> nobody noticed.  But we ought to fix it going forward.
> 
> I don't really like your solution though.  ISTM the intent of the
> code is that if the shared hashtable already exists, we adhere to the
> properties it has, we don't rely on the current caller to specify the
> exact same values.  So relying on the caller to get HASH_FIXED_SIZE
> correct here seems like the wrong thing.  I think we ought to add
> an isfixed flag to the shared hashtable header and copy that.
> (IOW, isfixed ought to act more like keysize/ssize/sshift, and should
> perhaps be grouped with them.)
> 
>             regards, tom lane

Thank's, I agree with you.
Attaching an edited version of the patch.

regards,
Aidar Imamov

Aidar Imamov <a.imamov@postgrespro.ru> writes:
> Thank's, I agree with you.
> Attaching an edited version of the patch.

Re-reading this in the light of morning, I realized that it can be
done even more simply: let's just move isfixed to the shared struct,
rather than keep two copies.  The argument for two copies of keysize
et al is that they're used in hot code paths.  But element_alloc()
had better not be a hot code path for a shared hash table, so I
don't think that argument has force for isfixed.

Pushed with that adjustment.  I didn't back-patch, because we've
not heard complaints about people running out of shmem on Windows.
If there is anybody running a workload where this would matter,
they might be less happy not more happy about the behavior
changing in a minor release.

            regards, tom lane