[BUG] test_lhash fails on NonStop (threading) at commit 01753c09bb

This test failure has just shown up. More info is coming:

02-test_lhash.t .........................
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 7166 Expected rc 0 on insert of element 15 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 7174 Expected rc 0 on insert of element 87 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 7182 Expected rc 0 on insert of element 91 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 7192 Expected rc 0 on insert of element 213 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 0 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 1 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 2 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 3 failed: Failure on insert
    #
    # OPENSSL_TEST_RAND_SEED=1721147672
    not ok 5 - test_hashtable_multithread
# ------------------------------------------------------------------------------
../../util/wrap.pl ../../test/lhash_test => 255

More info. It looks like a bad free() occurred.
02-test_lhash.t ..
# The results of this test will end up in test-runs/test_lhash
1..1
    # Subtest: ../../test/lhash_test
    1..5
    ok 1 - test_int_lhash
    ok 2 - test_stress
    ok 3 - test_int_hashtable
    ok 4 - test_hashtable_stress
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 8097 Expected rc 0 on insert of element 37 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 8101 Expected rc 0 on insert of element 155 which is in table
    #
\NS8NG8.$:2:671:86160236 - *** Run-time Error 034 ***
\NS8NG8.$:2:671:86160236 - Released space not allocated
../../util/wrap.pl ../../test/lhash_test => 159

Configuration (threading):
Command line (with current working directory = .):

    perl ./Configure nonstop-nsx_64_klt --prefix=/usr/local-ssl3.0 --openssldir=/usr/local-ssl3.0/ssl --with-rand-seed=rdcpu threads enable-fips -D_REENTRANT

Perl information:

    perl
    5.30.3 for NSV-D-nonstop_kernel

Enabled features:

    acvp-tests
    apps
    argon2
    aria
    async
    autoalginit
    autoerrinit
    autoload-config
    bf
    blake2
    bulk
    cached-fetch
    camellia
    capieng
    cast
    chacha
    cmac
    cmp
    cms
    comp
    ct
    default-thread-pool
    deprecated
    des
    dgram
    dh
    docs
    dsa
    dso
    dtls
    dynamic-engine
    ec
    ec2m
    ecdh
    ecdsa
    ecx
    egd
    engine
    err
    filenames
    fips
    fips-securitychecks
    gost
    http
    idea
    legacy
    loadereng
    md4
    mdc2
    module
    multiblock
    nextprotoneg
    ocb
    ocsp
    padlockeng
    pic
    pinshared
    poly1305
    posix-io
    psk
    quic
    unstable-qlog
    rc2
    rc4
    rdrand
    rfc3779
    rmd160
    scrypt
    secure-memory
    seed
    shared
    siphash
    siv
    sm2
    sm2-precomp
    sm3
    sm4
    sock
    srp
    srtp
    sse2
    ssl
    ssl-trace
    static-engine
    stdio
    tests
    thread-pool
    threads
    tls
    integrity-only-ciphers
    ts
    ui-console
    whirlpool
    tls1
    tls1-method
    tls1_1
    tls1_1-method
    tls1_2
    tls1_2-method
    tls1_3
    dtls1
    dtls1-method
    dtls1_2
    dtls1_2-method

Disabled features:

    afalgeng               [not-linux]      OPENSSL_NO_AFALGENG
    asan                   [default]        OPENSSL_NO_ASAN
    asm                    [no asm_arch]    OPENSSL_NO_ASM
    atexit                 [config]         OPENSSL_NO_ATEXIT
    brotli                 [default]        OPENSSL_NO_BROTLI
    brotli-dynamic         [default]        OPENSSL_NO_BROTLI_DYNAMIC
    buildtest-c++          [default]
    winstore               [not-windows]    OPENSSL_NO_WINSTORE
    crypto-mdebug          [default]        OPENSSL_NO_CRYPTO_MDEBUG
    demos                  [default]        OPENSSL_NO_DEMOS
    h3demo                 [default]        OPENSSL_NO_H3DEMO
    devcryptoeng           [default]        OPENSSL_NO_DEVCRYPTOENG
    ec_nistp_64_gcc_128    [default]        OPENSSL_NO_EC_NISTP_64_GCC_128
    external-tests         [default]        OPENSSL_NO_EXTERNAL_TESTS
    fuzz-afl               [default]        OPENSSL_NO_FUZZ_AFL
    fuzz-libfuzzer         [default]        OPENSSL_NO_FUZZ_LIBFUZZER
    ktls                   [default]        OPENSSL_NO_KTLS
    makedepend             [unavailable]
    md2                    [default]        OPENSSL_NO_MD2 (skip crypto/md2)
    msan                   [default]        OPENSSL_NO_MSAN
    pie                    [default]        OPENSSL_NO_PIE
    rc5                    [default]        OPENSSL_NO_RC5 (skip crypto/rc5)
    sctp                   [default]        OPENSSL_NO_SCTP
    tfo                    [default]        OPENSSL_NO_TFO
    trace                  [default]        OPENSSL_NO_TRACE
    ubsan                  [default]        OPENSSL_NO_UBSAN
    unit-test              [default]        OPENSSL_NO_UNIT_TEST
    uplink                 [no uplink_arch] OPENSSL_NO_UPLINK
    weak-ssl-ciphers       [default]        OPENSSL_NO_WEAK_SSL_CIPHERS
    zlib                   [default]        OPENSSL_NO_ZLIB
    zlib-dynamic           [default]        OPENSSL_NO_ZLIB_DYNAMIC
    zstd                   [default]        OPENSSL_NO_ZSTD
    zstd-dynamic           [default]        OPENSSL_NO_ZSTD_DYNAMIC
    ssl3                   [default]        OPENSSL_NO_SSL3
    ssl3-method            [default]        OPENSSL_NO_SSL3_METHOD

Config target attributes:

    AR => "ar",
    ARFLAGS => "qc",
    CC => "c99",
    HASHBANGPERL => "/usr/bin/env perl",
    RANLIB => "",
    RC => "windres",
    bin_cflags => "",
    bin_lflags => "",
    bn_ops => "SIXTY_FOUR_BIT",
    build_file => "Makefile",
    build_scheme => [ "unified", "unix" ],
    cflags => "-g -O2 -Wextensions -Wnowarn=203,220,272,734,770,1506 -Wbuild_neutral_library -Wverbose -Wtarget=tns/x -Wsystype=oss -Wlp64 -WIEEE_float",
    cppflags => "",
    defines => [ "OPENSSL_BUILDING_OPENSSL", "OPENSSL_VPROC=\$(OPENSSL_VPROC)", "_XOPEN_SOURCE", "_XOPEN_SOURCE_EXTENDED=1", "_TANDEM_SOURCE", "__NSK_OPTIONAL_TYPES__", "B_ENDIAN", "_KLT_MODEL_", "_REENTRANT", "_THREAD_SUPPORT_FUNCTIONS" ],
    disable => [ "atexit" ],
    dso_scheme => "DLFCN",
    enable => [ "egd" ],
    ex_libs => "-lrld -lklt",
    includes => [  ],
    lflags => "-Wxld=\"-set systype oss\" -Wxld=\"-set data_model lp64\" -Wxld=\"-set floattype ieee_float\"",
    lib_cflags => "",
    lib_cppflags => "",
    lib_defines => [  ],
    module_cflags => "",
    module_cxxflags => undef,
    module_ldflags => "-Wshared",
    multibin => "64-klt",
    multilib => "64-klt",
    perl => "/usr/bin/perl",
    perl_platform => "Unix",
    shared_argfileflag => "-Wxld_obey=",
    shared_cflag => "",
    shared_defflag => "-Wxld_obey=",
    shared_defines => [  ],
    shared_extension => ".so",
    shared_ldflag => "-Wshared",
    shared_rcflag => "",
    shared_target => "nonstop-shared",
    sys_id => "TANDEM",
    thread_defines => [  ],
    thread_scheme => "(unknown)",
    unistd => "<unistd.h>",

Recorded environment:

    AR =
    BUILDFILE =
    CC =
    CFLAGS =
    CPPFLAGS =
    CROSS_COMPILE =
    CXX =
    CXXFLAGS =
    HASHBANGPERL =
    LDFLAGS =
    LDLIBS =
    OPENSSL_LOCAL_CONFIG_DIR =
    PERL =
    RANLIB =
    RC =
    RCFLAGS =
    WINDRES =
    __CNF_CFLAGS =
    __CNF_CPPDEFINES =
    __CNF_CPPFLAGS =
    __CNF_CPPINCLUDES =
    __CNF_CXXFLAGS =
    __CNF_LDFLAGS =
    __CNF_LDLIBS =

Makevars:

    AR              = ar
    ARFLAGS         = qc
    ASFLAGS         =
    CC              = c99
    CFLAGS          =
    CPPDEFINES      = _REENTRANT
    CPPFLAGS        =
    CPPINCLUDES     =
    CXXFLAGS        =
    HASHBANGPERL    = /usr/bin/env perl
    LDFLAGS         =
    LDLIBS          =
    PERL            = perl
    RC              = windres
    RCFLAGS         =

NOTE: These variables only represent the configuration view.  The build file
template may have processed these variables further, please have a look at the
build file for more exact data:
    Makefile

build file:

    Makefile

build file templates:

    Configurations/common0.tmpl
    Configurations/unix-Makefile.tmpl

From the (virtually non-existent) stack trace, this looks like either a bad free in an atexit() call or the whole stack was overwritten or this is in a thread termination. Not much more I can get at. Assistance needed isolating this.

More: This fails the same way 1 out of every 2 or 3 runs. Maybe an uninitialized variable somewhere?

This appears to happen only when using threads. Thread global data is not initialized to 0 if that makes a difference in the diagnosis of this. The data is whatever happens to be in memory at the time. Running the test twice in the same CPU without anything else running seems to clear the issue because the memory used by the process is overwritten.

Another test run with different results. This looked like it passed -1 up to main() as the exit code.
[New thread 2]
[New thread 3]
[New thread 4]
[New thread 5]
[Thread 3 exited]
[Thread 5 exited]
[Thread 2 exited]

    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 9232 Expected rc 0 on insert of element 94 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 9235 Expected rc 0 on insert of element 107 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 9236 Expected rc 0 on insert of element 36 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 9237 Expected rc 0 on insert of element 105 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 0 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 1 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 2 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 3 failed: Failure on insert
    #
    # OPENSSL_TEST_RAND_SEED=1721152605
    not ok 5 - test_hashtable_multithread
# ------------------------------------------------------------------------------[Thread 4 exited]
Process (2,1077) exited with code 077777.
Removed process (2,1077).
xInspect is exiting...

../../util/wrap.pl ../../test/lhash_test => 255
not ok 1 - running lhash_test
02-test_lhash.t .. 1/? ---------------------------------------------------------
#   Failed test 'running lhash_test'
#   at /home/ituglib/randall/openssl-klt/util/perl/OpenSSL/Test/Simple.pm line 77.
02-test_lhash.t .. Dubious, test returned 1 (wstat 256, 0x100)

how much ram does this process have available to it?  Given that its failing on high iterations (around 9000), I'm wondering if you're seeing a failure in hash table growth.
The free error might occur because, on repeated growth, we appear to have a bug in which we don't free the correct pointer.  Try this patch:
diff --git a/crypto/hashtable/hashtable.c b/crypto/hashtable/hashtable.c
index 61d11940a9..ba546c7c15 100644
--- a/crypto/hashtable/hashtable.c
+++ b/crypto/hashtable/hashtable.c
@@ -470,7 +470,7 @@ static int grow_hashtable(HT *h, size_t oldsize)
             }
             if (rehashed == 0) {
                 /* we ran out of space in a neighborhood, grow again */
-                OPENSSL_free(newmd->neighborhoods);
+                OPENSSL_free(newmd->neighborhood_ptr_to_free);
                 OPENSSL_free(newmd);
                 return grow_hashtable(h, newsize);
             }

I expect that will cure any unallocated free issues.  Not sure what to do about out of memory conditions though.  We would have to start printing out allocation sizes

part of the issue with oversized allocations might be the implementation of CRYPTO_aligned_alloc.  If nonstop doesn't define _POSIX_C_SOURCE to a value greater than 200112L or _ISO_C11_SOURCE, you'll fall back to an aligned alloc implementation that allocates more data than needed to satisfy alignment requirements.  It appears that nonstop has an aligned_alloc call, so you may want to consider adjusting the defines in CRYPTO_aligned_alloc to ensure that that code path gets built (if its no already)

@nhorman I'm checking with the OS group on how much thread memory is available. Processes have loads. 64-bit address space and I have never seen a malloc fail on the box. The above patch made no difference. The use of _POSIX_C_SOURCE in system header files goes as far as 2008. Mostly, it is 2. However, the aligned_alloc call is only available in c11. I am going to try that compiler for the new thread model where this happens. Whatever caused this, which more recent than 3.3, would break our ability to provide PUT support, which uses c99.

Trouble... changing cc = 'c11' causes the following:
c99: error: Invalid input file extension"c11".

It seems that the configuration structure does not permit overwriting the first compiler setting, which is c99. I'm trying again only modifying the base setting, but this is not a long-term solution.

Did you mean to type cc (lowercase)?  I think the environment variable you want is CC (upper case), no?

@nhorman Sadly, we do not have posix_memalign, so c11 is not an option at all. In fact, we will have to modify the code to bypass this when we upgrade the minimum compiler, in about 1.5 years. I guess I'm stuck. The real question in my mind is why this just showed around this commit, as opposed to earlier 3.4 commits.

Did you mean to type cc (lowercase)? I think the environment variable you want is CC (upper case), no?

Lowercase is what is in the unix models and worked before.

Reworking the defines in crypto/mem.c to make this build better.

That I cant say, the commit you reference doesn't modify the hashtable code at all, so I'm at a bit of a loss.

Did you mean to type cc (lowercase)? I think the environment variable you want is CC (upper case), no?

Lowercase is what is in the unix models and worked before.

I opened a separate issue #24915 for the configuration override.

not 64 bit aligned, 64 byte aligned.  Its a performance thing, to aligned allocations to cache line size.  you can modify the hashtable code to just do a regular allocation if you like, but I'm not sure that it will help.
Interestingly, looking at the above failure (setting assigned the bad free, which I'm sure is being cause by using the non-aligned pointer address in the free operation, I'll fix that soon), The failures are all inserts, failing because we expect that an insert operation will fail on hash table conflict, but its actually getting inserted successfully.  It might be worth augmenting the  multithread lhash test code to print the index and key value getting used for every operation to give us a log of whats expected in the table up to the point that the error occurs

I think we managed to make this worse. The test is now transiently hanging in a hard loop. Sometimes it passes. Sometimes it fails as above. Always in subtest 5 (multithread).

On c99 again, back to alternating between PASS and FAIL. Hoping for your free fix (not judging quality based on "free" :) ).

can you try this patch:
diff --git a/crypto/hashtable/hashtable.c b/crypto/hashtable/hashtable.c
index 61d11940a9..b91501fdb4 100644
--- a/crypto/hashtable/hashtable.c
+++ b/crypto/hashtable/hashtable.c
@@ -470,7 +470,7 @@ static int grow_hashtable(HT *h, size_t oldsize)
             }
             if (rehashed == 0) {
                 /* we ran out of space in a neighborhood, grow again */
-                OPENSSL_free(newmd->neighborhoods);
+                OPENSSL_free(newmd->neighborhood_ptr_to_free);
                 OPENSSL_free(newmd);
                 return grow_hashtable(h, newsize);
             }
@@ -497,7 +497,7 @@ static int grow_hashtable(HT *h, size_t oldsize)
 out:
     return rc;
 out_free:
-    OPENSSL_free(newmd->neighborhoods);
+    OPENSSL_free(newmd->neighborhood_ptr_to_free);
     OPENSSL_free(newmd);
     goto out;
 }

Sadly this yields a the same (or similar) transient failure:
02-test_lhash.t ..
# The results of this test will end up in test-runs/test_lhash
1..1
    # Subtest: ../../test/lhash_test
    1..5
    ok 1 - test_int_lhash
    ok 2 - test_stress
    ok 3 - test_int_hashtable
    ok 4 - test_hashtable_stress
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 15524 Expected rc 0 on insert of element 84 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 15527 Expected rc 0 on insert of element 241 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 15528 Expected rc 0 on insert of element 99 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 15530 Expected rc 0 on insert of element 43 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 0 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 1 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 2 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 3 failed: Failure on insert
    #
    # OPENSSL_TEST_RAND_SEED=1721221570
    not ok 5 - test_hashtable_multithread
../../util/wrap.pl ../../test/lhash_test => 255
not ok 1 - running lhash_test

#   Failed test 'running lhash_test'
#   at /home/ituglib/randall/openssl-klt/util/perl/OpenSSL/Test/Simple.pm line 77.
# Looks like you failed 1 test of 1.
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests

Need diagnostics inside? I can put stuff in but guidance on what would be appreciated.

Ok, but that change was specifically meant to prevent the crash that you reported here:
https://github.com/openssl/openssl/issues/24913#issuecomment-2231404649'
And it seems that is no longer occuring (I'm assuming given that your output above shows no crash)
As for the augmented output, I would suggest the following:
diff --git a/test/lhash_test.c b/test/lhash_test.c
index 76bed5d7e5..0123d74de2 100644
--- a/test/lhash_test.c
+++ b/test/lhash_test.c
@@ -560,6 +560,8 @@ static void do_mt_hash_work(void)
         switch(behavior) {
         case DO_LOOKUP:
             ossl_ht_read_lock(m_ht);
+            fprintf(stderr, "Doing lookup on entry %d, expected m is %p\n",
+                            index, expected_m);
             m = ossl_ht_mt_TEST_MT_ENTRY_get(m_ht, TO_HT_KEY(&key), &v);
             if (m != NULL && m != expected_m) {
                 worker_exits[num] = "Read unexpected value from hashtable";
@@ -581,6 +583,7 @@ static void do_mt_hash_work(void)
                 r = NULL;
             }

+            fprintf(stderr, "Doing %s on index %d(%p), expected rc is %d\n", behavior == DO_REPLACE ? "Replace" : "Insert", index, expected_m, expected_rc);
             if (expected_rc != ossl_ht_mt_TEST_MT_ENTRY_insert(m_ht,
                                                                TO_HT_KEY(&key),
                                                                expected_m, r)) {
@@ -598,6 +601,8 @@ static void do_mt_hash_work(void)
         case DO_DELETE:
             ossl_ht_write_lock(m_ht);
             expected_rc = expected_m->in_table;
+            fprintf(stderr, "Doing Delete on index %d(%p), expected_rc is %d\n",
+                index, expected_m, expected_rc);
             if (expected_rc != ossl_ht_delete(m_ht, TO_HT_KEY(&key))) {
                 TEST_info("Iteration %d Expected rc %d on delete of element %d which is %s\n",
                           giter, expected_rc, index,

well, I'm not sure what to tell you there.  This test gets run under threadsan and asan regularly, and no errors have popped up. granted those are run under linux and gcc/clang, and not nonstop, but if there is a memory error, its not getting detected on other platforms.
To me, given your outline above, this seems to me like a co-operative threading problem perhaps.  With the use of rcu locks, there are several paths which shouldn't really ever yield the cpu.
Can you post the output from the test conducted in step 6?

well, I'm not sure what to tell you there. This test gets run under threadsan and asan regularly, and no errors have popped up. granted those are run under linux and gcc/clang, and not nonstop, but if there is a memory error, its not getting detected on other platforms.
To me, given your outline above, this seems to me like a co-operative threading problem perhaps. With the use of rcu locks, there are several paths which shouldn't really ever yield the cpu.
Can you post the output from the test conducted in step 6?

Actually, this new threading model is preemptive not cooperative, which is why things seem to be overall working better (and faster). CPU yields should not be an issue, but each thread is in its own distinct core. I have reported the transient failures as a potential issue associated with the thread model (long-shot guess). The last test is:
# The results of this test will end up in test-runs/test_lhash
1..1
    # Subtest: ../../test/lhash_test
    1..5
    ok 1 - test_int_lhash
    ok 2 - test_stress
    ok 3 - test_int_hashtable
    ok 4 - test_hashtable_stress
    ok 5 - test_hashtable_multithread
../../util/wrap.pl ../../test/lhash_test => 0
ok 1 - running lhash_test
ok
All tests successful.

Even more weirdly is that some other passing tests are generating more output. For example (2 runs back from the above):
# The results of this test will end up in test-runs/test_lhash
1..1
    # Subtest: ../../test/lhash_test
    1..5
    ok 1 - test_int_lhash
    ok 2 - test_stress
    ok 3 - test_int_hashtable
    ok 4 - test_hashtable_stress
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 22233 Expected rc 0 on insert of element 243 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 22235 Expected rc 0 on insert of element 181 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 22239 Expected rc 0 on insert of element 23 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:604
    # Iteration 22248 Expected rc 0 on delete of element 127 which is not in table
    #
../../util/wrap.pl ../../test/lhash_test => 0
ok 1 - running lhash_test
ok
All tests successful.

Both tests ran identically. By changing the root CPU, I can more often get a failure, and then a series of passes. The configuration is 4 CPUs and each CPU has 6 cores. Threads will have preference within the cores of the same CPU, but that is not guaranteed.

I don't see any of the printf output in the above.  Can you run with V=1 to ensure it makes it to the console?

I don't see any of the printf output in the above. Can you run with V=1 to ensure it makes it to the console?

That was run with V=1 without the fprintf added, which seem to consistently suppress the failures.

can you just run it as:
./test/lhash_test

It should be statically linked, so you shouldn't have to worry about library paths or anything

Will try when back in office shortly. It's not got atexit() does it?

no, atexit should definately not be in play here.  A cleanup handler gets registered, but all threads are joined prior to the application exiting, and the test only exercises the hash table, so I'd be hard pressed to see any issues here.  You're welcome to build with no-atexit to check, but I don't think it will change anything

./test/lhash_test
1..5
ok 1 - test_int_lhash
ok 2 - test_stress
ok 3 - test_int_hashtable
ok 4 - test_hashtable_stress
<<hangs>>

When run that way, failures are pretty consistent but the messages logged are not. Weird.

no output at all?  Thats really odd.

Here is the output of multiple runs.
./test/lhash_test
1..5
ok 1 - test_int_lhash
ok 2 - test_stress
ok 3 - test_int_hashtable
ok 4 - test_hashtable_stress
HANG

./test/lhash_test
1..5
ok 1 - test_int_lhash
ok 2 - test_stress
ok 3 - test_int_hashtable
ok 4 - test_hashtable_stress
# INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
# Iteration 3007 Expected rc 0 on insert of element 89 which is in table
#
# INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
# Iteration 3013 Expected rc 0 on insert of element 230 which is in table
#
# INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
# Iteration 3017 Expected rc 0 on insert of element 146 which is in table
#
# INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
# Iteration 3025 Expected rc 0 on insert of element 219 which is in table
#
# INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
# Worker 0 failed: Failure on insert
#
# INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
# Worker 1 failed: Failure on insert
#
# INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
# Worker 2 failed: Failure on insert
#
# INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
# Worker 3 failed: Failure on insert
#
# OPENSSL_TEST_RAND_SEED=1721233273
not ok 5 - test_hashtable_multithread
FAIL

./test/lhash_test
1..5
ok 1 - test_int_lhash
ok 2 - test_stress
ok 3 - test_int_hashtable
ok 4 - test_hashtable_stress
# INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
# Iteration 11697 Expected rc 0 on insert of element 63 which is in table
#
HANG

./test/lhash_test
1..5
ok 1 - test_int_lhash
ok 2 - test_stress
ok 3 - test_int_hashtable
ok 4 - test_hashtable_stress
ok 5 - test_hashtable_multithread
PASS

I'm still lost as to why the stderr printfs aren't being produced here.  Especially when running in the manner that you are.  You could try converting the fprintf call to a TEST_info call, since clearly those are getting produced, but theres no reason that fprintf shouldn't produce anything here.  Without that, its going to be really hard to know what the hashtable state history was.

I'm still lost as to why the stderr printfs aren't being produced here. Especially when running in the manner that you are. You could try converting the fprintf call to a TEST_info call, since clearly those are getting produced, but theres no reason that fprintf shouldn't produce anything here. Without that, its going to be really hard to know what the hashtable state history was.

Sorry, I removed them because with them, the test never failed. Converting to TEST_info would be good. I will try that.

@nhorman Last segment of the test... output is pretty large. This one was finally a FAIL with the diagnostics, so yay.

The first failure that I see is this:

# INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:590
    # Doing Insert on index 103(0x80d2a08), expected rc is 0
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:597
    # Iteration 19078 Expected rc 0 on insert of element 103 which is in table

looking back through your log for references to index 103 I see:
 # INFO: INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:590 @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:565
    #
    # Doing Insert on index 103(0x80d2a08), expected rc is 0
    # Doing lookup on entry 247, expected m is 0x80d2e88
...

# INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:590
    # Doing Insert on index 103(0x80d2a08), expected rc is 0
    #
...
# INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:590
    # Doing Insert on index 103(0x80d2a08), expected rc is 0
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:597
    # Iteration 19078 Expected rc 0 on insert of element 103 which is in table
...

So we can see that we have several prior iterations in the logs in which we attempt to insert an element to the hash table that is already inserted, and the operation fails, as expected.  Until iteration 19078, in which it all of a sudden passes, without a prior delete operation to remove it.  Assuming for the moment that the insert of this element happened somewhere within the lost logs, thats...really wierd.
I would agree at this point that something has done an improper memory write in this test, as you said previously, as the value of in_table for this array entry has been modified without an express delete operation, as you hypothesized previously.  But that seems to stand in contrast to our regular CI tests which don't detect any thread or asan errors.
So, I can only conclude right now that we have a potential memory error that is only occuring on NonStop.  Does the platform have anything similar to libasan that we can use to track this down?

for completeness attaching the file would be helpful yes, if only to confirm that the missing entries were in fact added.
Another thing to look for would be to add some debugging to ensure that on state altering operations (insert/replace/delete) the locks are actually being held properly.  I can write that up for you in the am

heres a patch to do some more debugging.  It includes the previous changes, plus some to help catch failures in locking.  You'll need to configure openssl with -d to enable debug asserts:
diff --git a/crypto/hashtable/hashtable.c b/crypto/hashtable/hashtable.c
index 61d11940a9..c31848640a 100644
--- a/crypto/hashtable/hashtable.c
+++ b/crypto/hashtable/hashtable.c
@@ -250,6 +250,10 @@ void ossl_ht_read_unlock(HT *htable)
 void ossl_ht_write_lock(HT *htable)
 {
     ossl_rcu_write_lock(htable->lock);
+    if (htable->wpd.need_sync != 0) {
+        fprintf(stderr, "NEED SYNC IS MISSET!\n");
+        fprintf(stderr, "Intentionally crashing! %s\n", NULL);
+    }
     htable->wpd.need_sync = 0;
 }

diff --git a/crypto/threads_pthread.c b/crypto/threads_pthread.c
index 7b54599947..b114b0d745 100644
--- a/crypto/threads_pthread.c
+++ b/crypto/threads_pthread.c
@@ -354,6 +354,7 @@ struct rcu_lock_st {

     /* signal to wake threads waiting on prior_lock */
     pthread_cond_t prior_signal;
+    int sentry;
 };

 /* Read side acquisition of the current qp */
@@ -568,12 +569,16 @@ static struct rcu_qp *allocate_new_qp_group(CRYPTO_RCU_LOCK *lock,
 void ossl_rcu_write_lock(CRYPTO_RCU_LOCK *lock)
 {
     pthread_mutex_lock(&lock->write_lock);
+    assert(lock->sentry == 0);
+    lock->sentry = 1;
     TSAN_FAKE_UNLOCK(&lock->write_lock);
 }

 void ossl_rcu_write_unlock(CRYPTO_RCU_LOCK *lock)
 {
     TSAN_FAKE_LOCK(&lock->write_lock);
+    assert(lock->sentry == 1);
+    lock->sentry = 0;
     pthread_mutex_unlock(&lock->write_lock);
 }

diff --git a/test/lhash_test.c b/test/lhash_test.c
index 76bed5d7e5..0123d74de2 100644
--- a/test/lhash_test.c
+++ b/test/lhash_test.c
@@ -560,6 +560,8 @@ static void do_mt_hash_work(void)
         switch(behavior) {
         case DO_LOOKUP:
             ossl_ht_read_lock(m_ht);
+            fprintf(stderr, "Doing lookup on entry %d, expected m is %p\n",
+                            index, expected_m);
             m = ossl_ht_mt_TEST_MT_ENTRY_get(m_ht, TO_HT_KEY(&key), &v);
             if (m != NULL && m != expected_m) {
                 worker_exits[num] = "Read unexpected value from hashtable";
@@ -581,6 +583,7 @@ static void do_mt_hash_work(void)
                 r = NULL;
             }

+            fprintf(stderr, "Doing %s on index %d(%p), expected rc is %d\n", behavior == DO_REPLACE ? "Replace" : "Insert", index, expected_m, expected_rc);
             if (expected_rc != ossl_ht_mt_TEST_MT_ENTRY_insert(m_ht,
                                                                TO_HT_KEY(&key),
                                                                expected_m, r)) {
@@ -598,6 +601,8 @@ static void do_mt_hash_work(void)
         case DO_DELETE:
             ossl_ht_write_lock(m_ht);
             expected_rc = expected_m->in_table;
+            fprintf(stderr, "Doing Delete on index %d(%p), expected_rc is %d\n",
+                index, expected_m, expected_rc);
             if (expected_rc != ossl_ht_delete(m_ht, TO_HT_KEY(&key))) {
                 TEST_info("Iteration %d Expected rc %d on delete of element %d which is %s\n",
                           giter, expected_rc, index,

Murphy's law... cannot get the thing to fail on demand... will apply your changes above as TEST_info and retry it again and again.

Full log with failure.
openssl.lhash.log

Full log with changes above from @nhorman. This one passes. I will keep rerunning.
openssl.lhash.log

Full log ith changes above from @nhorman. This one is a fail log.
openssl.lhash.log

Thank you.  Looking over those, I can confirm that the state for the failing elements is consistent right up to the point that its not (ie no prior unexpected transitions).  Additionally the locking doesn't seem to flag any errors (I assume you build with -d), as there are no assert failures.
I'm trying to come up with a new hypothesis for these failures, given that the above locking checks all passed.  What I see is:
4 elements failed:
# INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:597
    # Iteration 15174 Expected rc 0 on insert of element 34 which is in table
    #
...
# INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:597
    # Iteration 15180 Expected rc 0 on insert of element 112 which is in table
    #
...
# INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:597
    # Iteration 15182 Expected rc 0 on insert of element 54 which is in table
    #
...
# INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:597
    # Iteration 15198 Expected rc 0 on insert of element 212 which is in table
    #

All of those indices (112, 212, 54 and 34) had been operated on several times successfully and their state transitions worked just fine...until they didn't, which implies that something unrelated to operations on those indices caused them to change.
Hypothesis:  we have a pointer assignment type issue - i.e. perhaps somewhere within the hash table we are assuming that a void* pointer (which is nominally used to store HT_VALUE structs in the hash table is actually being treated as a uint64_t or some such, causing write overruns.  I note that addresses for the expected table in the test report as 32 bit, which implies these are all 32 bit processors (is this correct @rsbeckerca ?).  If so, this doesn't explain why we don't see the failure on 32 bit builds, but it might explain why asan doesn't catch them (as we are still modifying allocated memory)
The test would be easy to add I think, we would just need to compare the hashtable neighborhood before and after a delete or insert operation, and crash on a failed 0 comparison.  I can write up a test for that.
Hypothesis: CPU caching gone awry.  I don't know how mesi works on Non-stop system, but is cache coherency weak on NonStop platforms by any chance @rsbeckerca ?  i.e. if a thread migrates to a different cpu, is it possible that it sees an old version of memory?  something similar to what you might see on ARM?  I'm really not sure how to test this, but it seems like it might be worth following up on

I did not build with -dNDEBUG (what you meant?). Will redo the test, trying to keep the optimization level the same.
The build is 64 bit, so I'm not sure what is going on there. The addresses will start at 32-bit values based on magnitude but part of the heap will eventually go to larger addresses.
As far as I understand the threading implementation, once a core is assigned to a thread, the thread should never move. There is no "migration" to another CPU. In fact, there is a strong reason in the architecture to prohibit migration as there are process and thread identifiers that depend on the CPU remaining permanently assigned (basically internal process handle encodes the CPU in it and unique way to communicate with the process if necessary (any process can talk to any other assuming the destination is listening and security allows it) without the need for pesky sockets or pipes (goes back to platform initial design in 1974). It is possible to migrate a top level process to a different core, but you need root to do that. I don't think threads migrate to different cores but they would stay in the same CPU if they did. Memory is shared across cores so I suppose the thread library could technically do that move if a core was overloaded. That does not happen in PUT where the assigned single CPU+core cannot be dynamic.
If CPU caching has gone awry, this would have broken a lot more than we are seeing. The OS version would never have left the developer's desk.

The debug switch is a bit confusing.  assert() causes a crash if -DNDEBUG is not defined, and the build automatically defined -DNDEBUG if the -d option is not specified during configuration, so if you run ./Configure -d, the asserts should work as expected.

new patch:
diff --git a/crypto/hashtable/hashtable.c b/crypto/hashtable/hashtable.c
index 61d11940a9..ccaf579d17 100644
--- a/crypto/hashtable/hashtable.c
+++ b/crypto/hashtable/hashtable.c
@@ -47,6 +47,7 @@
  */

 #include <string.h>
+#include <assert.h>
 #include <internal/rcu.h>
 #include <internal/hashtable.h>
 #include <openssl/rand.h>
@@ -250,6 +251,10 @@ void ossl_ht_read_unlock(HT *htable)
 void ossl_ht_write_lock(HT *htable)
 {
     ossl_rcu_write_lock(htable->lock);
+    if (htable->wpd.need_sync != 0) {
+        fprintf(stderr, "NEED SYNC IS MISSET!\n");
+        fprintf(stderr, "Intentionally crashing! %s\n", NULL);
+    }
     htable->wpd.need_sync = 0;
 }

@@ -525,6 +530,8 @@ static int ossl_ht_insert_locked(HT *h, uint64_t hash,
     uint64_t ihash;
     HT_VALUE *ival;
     size_t empty_idx = SIZE_MAX;
+    struct ht_neighborhood_entry_st check[NEIGHBORHOOD_LEN];
+    struct ht_neighborhood_entry_st check_next[NEIGHBORHOOD_LEN];

     PREFETCH_NEIGHBORHOOD(md->neighborhoods[neigh_idx]);

@@ -539,6 +546,14 @@ static int ossl_ht_insert_locked(HT *h, uint64_t hash,
                 /* invalid */
                 return 0;
             }
+            /* make a copy of the neighborhood */
+            memcpy(check, &md->neighborhoods[neigh_idx].entries,
+                   sizeof(struct ht_neighborhood_entry_st) * NEIGHBORHOOD_LEN);
+            /* also copy the next neighborhood if we're not last in the list */
+            if (neigh_idx != md->neighborhood_mask)
+                memcpy(check_next, &md->neighborhoods[neigh_idx+1].entries,
+                       sizeof(struct ht_neighborhood_entry_st) * NEIGHBORHOOD_LEN);
+
             /* Do a replacement */
             CRYPTO_atomic_store(&md->neighborhoods[neigh_idx].entries[j].hash,
                                 hash, h->atomic_lock);
@@ -546,6 +561,19 @@ static int ossl_ht_insert_locked(HT *h, uint64_t hash,
             ossl_rcu_assign_ptr(&md->neighborhoods[neigh_idx].entries[j].value,
                                 &newval);
             ossl_rcu_call(h->lock, free_old_ht_value, *olddata);
+
+            /* check to make sure that nothing else got changed */
+            if (memcmp(&md->neighborhoods[neigh_idx].entries[j+1], &check[j+1],
+                       (NEIGHBORHOOD_LEN - (j+1))*sizeof(struct ht_neighborhood_entry_st))) {
+                    fprintf(stderr, "FAILED NEIGHBORHOOD VALIDATION ON REPLACE\n");
+                    assert(0);
+            }
+            if (neigh_idx != md->neighborhood_mask &&
+                memcmp(check_next, &md->neighborhoods[neigh_idx+1].entries,
+                       NEIGHBORHOOD_LEN * sizeof(struct ht_neighborhood_entry_st))) {
+                    fprintf(stderr, "FAILED NEXT NEIGHBORHOOD VALIDATION ON REPLACE\n");
+                    assert(0);
+            }
             h->wpd.need_sync = 1;
             return 1;
         }
@@ -554,10 +582,31 @@ static int ossl_ht_insert_locked(HT *h, uint64_t hash,
     if (empty_idx == SIZE_MAX)
         return -1; /* out of space */
     h->wpd.value_count++;
+    /* make a copy of the neighborhood */
+        memcpy(check, &md->neighborhoods[neigh_idx].entries,
+               sizeof(struct ht_neighborhood_entry_st) * NEIGHBORHOOD_LEN);
+    /* also copy the next neighborhood if we're not last in the list */
+    if (neigh_idx != md->neighborhood_mask)
+        memcpy(check_next, &md->neighborhoods[neigh_idx+1].entries,
+               sizeof(struct ht_neighborhood_entry_st) * NEIGHBORHOOD_LEN);
+
     CRYPTO_atomic_store(&md->neighborhoods[neigh_idx].entries[empty_idx].hash,
                         hash, h->atomic_lock);
     ossl_rcu_assign_ptr(&md->neighborhoods[neigh_idx].entries[empty_idx].value,
                         &newval);
+    /* check to make sure that nothing else got changed */
+    if (memcmp(&md->neighborhoods[neigh_idx].entries[empty_idx+1], &check[empty_idx+1],
+           (NEIGHBORHOOD_LEN - (empty_idx+1))*sizeof(struct ht_neighborhood_entry_st))) {
+            fprintf(stderr, "FAILED NEIGHBORHOOD VALIDATION ON INSERT\n");
+            assert(0);
+    }
+    if (neigh_idx != md->neighborhood_mask &&
+        memcmp(check_next, &md->neighborhoods[neigh_idx+1].entries,
+                NEIGHBORHOOD_LEN * sizeof(struct ht_neighborhood_entry_st))) {
+            fprintf(stderr, "FAILED NEXT NEIGHBORHOOD VALIDATION ON INSERT\n");
+            assert(0);
+    }
+
     return 1;
 }

@@ -664,6 +713,8 @@ int ossl_ht_delete(HT *h, HT_KEY *key)
     struct ht_internal_value_st *v = NULL;
     HT_VALUE *nv = NULL;
     int rc = 0;
+    struct ht_neighborhood_entry_st check[NEIGHBORHOOD_LEN];
+    struct ht_neighborhood_entry_st check_next[NEIGHBORHOOD_LEN];

     hash = h->config.ht_hash_fn(key->keybuf, key->keysize);

@@ -672,11 +723,31 @@ int ossl_ht_delete(HT *h, HT_KEY *key)
     for (j = 0; j < NEIGHBORHOOD_LEN; j++) {
         if (compare_hash(hash, h->md->neighborhoods[neigh_idx].entries[j].hash)) {
             h->wpd.value_count--;
+            /* make a copy of the neighborhood */
+            memcpy(check, &h->md->neighborhoods[neigh_idx].entries,
+                   sizeof(struct ht_neighborhood_entry_st) * NEIGHBORHOOD_LEN);
+            /* also copy the next neighborhood if we're not last in the list */
+            if (neigh_idx != h->md->neighborhood_mask)
+                memcpy(check_next, &h->md->neighborhoods[neigh_idx+1].entries,
+                       sizeof(struct ht_neighborhood_entry_st) * NEIGHBORHOOD_LEN);
+
             CRYPTO_atomic_store(&h->md->neighborhoods[neigh_idx].entries[j].hash,
                                 0, h->atomic_lock);
             v = (struct ht_internal_value_st *)h->md->neighborhoods[neigh_idx].entries[j].value;
             ossl_rcu_assign_ptr(&h->md->neighborhoods[neigh_idx].entries[j].value,
                                 &nv);
+            /* check to make sure that nothing else got changed */
+            if (memcmp(&h->md->neighborhoods[neigh_idx].entries[j+1], &check[j+1],
+                       (NEIGHBORHOOD_LEN - (j+1))*sizeof(struct ht_neighborhood_entry_st))) {
+                    fprintf(stderr, "FAILED NEIGHBORHOOD VALIDATION ON DELETE\n");
+                    assert(0);
+            }
+            if (neigh_idx != h->md->neighborhood_mask &&
+                memcmp(check_next, &h->md->neighborhoods[neigh_idx+1].entries,
+                       NEIGHBORHOOD_LEN * sizeof(struct ht_neighborhood_entry_st))) {
+                    fprintf(stderr, "FAILED NEXT NEIGHBORHOOD VALIDATION ON DELETE\n");
+                    assert(0);
+            }
             rc = 1;
             break;
         }
diff --git a/crypto/threads_pthread.c b/crypto/threads_pthread.c
index 7b54599947..b114b0d745 100644
--- a/crypto/threads_pthread.c
+++ b/crypto/threads_pthread.c
@@ -354,6 +354,7 @@ struct rcu_lock_st {

     /* signal to wake threads waiting on prior_lock */
     pthread_cond_t prior_signal;
+    int sentry;
 };

 /* Read side acquisition of the current qp */
@@ -568,12 +569,16 @@ static struct rcu_qp *allocate_new_qp_group(CRYPTO_RCU_LOCK *lock,
 void ossl_rcu_write_lock(CRYPTO_RCU_LOCK *lock)
 {
     pthread_mutex_lock(&lock->write_lock);
+    assert(lock->sentry == 0);
+    lock->sentry = 1;
     TSAN_FAKE_UNLOCK(&lock->write_lock);
 }

 void ossl_rcu_write_unlock(CRYPTO_RCU_LOCK *lock)
 {
     TSAN_FAKE_LOCK(&lock->write_lock);
+    assert(lock->sentry == 1);
+    lock->sentry = 0;
     pthread_mutex_unlock(&lock->write_lock);
 }

diff --git a/test/lhash_test.c b/test/lhash_test.c
index 76bed5d7e5..0123d74de2 100644
--- a/test/lhash_test.c
+++ b/test/lhash_test.c
@@ -560,6 +560,8 @@ static void do_mt_hash_work(void)
         switch(behavior) {
         case DO_LOOKUP:
             ossl_ht_read_lock(m_ht);
+            fprintf(stderr, "Doing lookup on entry %d, expected m is %p\n",
+                            index, expected_m);
             m = ossl_ht_mt_TEST_MT_ENTRY_get(m_ht, TO_HT_KEY(&key), &v);
             if (m != NULL && m != expected_m) {
                 worker_exits[num] = "Read unexpected value from hashtable";
@@ -581,6 +583,7 @@ static void do_mt_hash_work(void)
                 r = NULL;
             }

+            fprintf(stderr, "Doing %s on index %d(%p), expected rc is %d\n", behavior == DO_REPLACE ? "Replace" : "Insert", index, expected_m, expected_rc);
             if (expected_rc != ossl_ht_mt_TEST_MT_ENTRY_insert(m_ht,
                                                                TO_HT_KEY(&key),
                                                                expected_m, r)) {
@@ -598,6 +601,8 @@ static void do_mt_hash_work(void)
         case DO_DELETE:
             ossl_ht_write_lock(m_ht);
             expected_rc = expected_m->in_table;
+            fprintf(stderr, "Doing Delete on index %d(%p), expected_rc is %d\n",
+                index, expected_m, expected_rc);
             if (expected_rc != ossl_ht_delete(m_ht, TO_HT_KEY(&key))) {
                 TEST_info("Iteration %d Expected rc %d on delete of element %d which is %s\n",
                           giter, expected_rc, index,

The above patch should test the latest hypothesis.  It makes a copy of the hashtable neighborhood that we are inserting into.  For the element we insert to, it does a memory comparison of all elements subsequent to the element being modified (on insert or delete), as well as the next ordinal neighborhood.  There should not be any changes to elements other than the one we decide to modify, so if the test fails, it should crash immediately, pointing to the specific call stack during which the erroneous change happened.  Fingers crossed

This one, built as above with the patch, shows a hang situation.
openssl.lhash.log

This one fails took about 6 iterations to get there...which is an interesting number (# cores/CPU) but I am not suggesting that is the cause, just curious.
openssl.lhash.log

can you obtain a backtrace?

can you obtain a backtrace?

Not a chance at this point. The external program that gets back-traces is not thread-aware, so it is likely to be in the dispatch code or waiting in main.

Here is what I have... looks like thread 4 never terminates
Procedure call trace:
  AsmBreakDispatchNoRelink + 0x2 (Milli)
  $n_NSK_Event_Wait + 0xCA (Milli)
  Timer_WaitOnEmbeddedTLE_ + 0xDE (SLr)
  PROCESS_WAIT_int___FUiLb + 0x95 (SLr)
  PROCESS_LONG_WAIT_ + 0x5B (SLr)
  KLT_join_ + 0x1F3 (SLr)
  $n_EnterPriv + 0x542 (Milli)
  pthread_join + 0x182 (DLL wkltdll)
  wait_for_thread + 0x38 (UCr)
  test_hashtable_multithread + 0x1D7 (UCr)
  run_tests + 0x491 (UCr)
  main + 0xAE (UCr)
  _MAIN + 0x3F (UCr)

The executing thread that appears never to terminate is below. The len on alloc is well within the capability of the system, so I seriously doubt that is the problem.
#0  0xfffffffff12766bb in memsetHP ()
#1  0xfffffffff1281770 in memset ()
#2  0x7015b213 in alloc_new_neighborhood_list (len=33554432,
    freeptr=0x100069df8)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:172
#3  0x7015c2ad in grow_hashtable (h=0x100024d50, oldsize=16777216)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:446
#4  0x7015c559 in grow_hashtable (h=0x100024d50, oldsize=8388608)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:480
#5  0x7015c559 in grow_hashtable (h=0x100024d50, oldsize=4194304)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:480
#6  0x7015c559 in grow_hashtable (h=0x100024d50, oldsize=2097152)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:480
#7  0x7015c559 in grow_hashtable (h=0x100024d50, oldsize=1048576)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:480
#8  0x7015c559 in grow_hashtable (h=0x100024d50, oldsize=524288)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:480
#9  0x7015c559 in grow_hashtable (h=0x100024d50, oldsize=262144)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:480
#10 0x7015c559 in grow_hashtable (h=0x100024d50, oldsize=131072)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:480
#11 0x7015c559 in grow_hashtable (h=0x100024d50, oldsize=65536)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:480
#12 0x7015c559 in grow_hashtable (h=0x100024d50, oldsize=32768)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:480
#13 0x7015c559 in grow_hashtable (h=0x100024d50, oldsize=16384)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:480
#14 0x7015c559 in grow_hashtable (h=0x100024d50, oldsize=8192)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:480
#15 0x7015c559 in grow_hashtable (h=0x100024d50, oldsize=4096)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:480
#16 0x7015c559 in grow_hashtable (h=0x100024d50, oldsize=2048)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:480
#17 0x7015c559 in grow_hashtable (h=0x100024d50, oldsize=1024)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:480
#18 0x7015c559 in grow_hashtable (h=0x100024d50, oldsize=512)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:480
#19 0x7015c559 in grow_hashtable (h=0x100024d50, oldsize=256)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:480
#20 0x7015c559 in grow_hashtable (h=0x100024d50, oldsize=128)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:480
#21 0x7015c559 in grow_hashtable (h=0x100024d50, oldsize=64)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:480
#22 0x7015d05d in ossl_ht_insert (h=0x100024d50, key=0x67e3fe10,
    data=0x67e3fd60, olddata=0x67e3fd70)
    at /home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c:661
#23 0x700e9a5f in ossl_ht_mt_TEST_MT_ENTRY_insert (h=0x100024d50,
    key=0x67e3fe10, data=0x80d58e8, olddata=0x67e3fdf8)
    at /home/ituglib/randall/openssl-klt/test/lhash_test.c:486
#24 0x700ea0ff in do_mt_hash_work ()
    at /home/ituglib/randall/openssl-klt/test/lhash_test.c:592
#25 0x700e77ec in thread_run (arg=0x700e9c80)
    at /home/ituglib/randall/openssl-klt/test/threadstest.h:67
#26 0xfffffffff4e95585 in thread_main ()
#27 0xfffffffff0c91f99 in __KLT_MINOR_ ()

well, thats definately telling.  We appear to be getting a repeated hashtable growth on collisions, which honestly, for this test, shouldn't be happening, but here we are.
The easy fix is to limit the number of times we can grow the hashtable, but that just leads to an earlier error, and given that we should have a fixed set of hash values here, theres no reason such growth shouldn't appear on other arches as well.
I'm curious to know now what hash values this test is coming up with.  so lets both limit hash table growth and print the hash values we are building out
New patch, it will only let you grow twice before failing, and it will also print out the hash values of the keys you are adding.  I'm starting to wonder if theres a problem in the hash value computation.
diff --git a/crypto/hashtable/hashtable.c b/crypto/hashtable/hashtable.c
index 61d11940a9..8a9e4515b3 100644
--- a/crypto/hashtable/hashtable.c
+++ b/crypto/hashtable/hashtable.c
@@ -47,6 +47,7 @@
  */

 #include <string.h>
+#include <assert.h>
 #include <internal/rcu.h>
 #include <internal/hashtable.h>
 #include <openssl/rand.h>
@@ -89,6 +90,7 @@ static ossl_unused uint64_t fnv1a_hash(uint8_t *key, size_t len)
         hash ^= key[i];
         hash *= 0x00000100000001B3ULL;
     }
+    fprintf(stderr, "Hash value computes to %llu\n", hash);
     return hash;
 }

@@ -250,6 +252,10 @@ void ossl_ht_read_unlock(HT *htable)
 void ossl_ht_write_lock(HT *htable)
 {
     ossl_rcu_write_lock(htable->lock);
+    if (htable->wpd.need_sync != 0) {
+        fprintf(stderr, "NEED SYNC IS MISSET!\n");
+        fprintf(stderr, "Intentionally crashing! %s\n", NULL);
+    }
     htable->wpd.need_sync = 0;
 }

@@ -423,7 +429,7 @@ static void free_old_neigh_table(void *arg)
  * Increase hash table bucket list
  * must be called with write_lock held
  */
-static int grow_hashtable(HT *h, size_t oldsize)
+static int grow_hashtable(HT *h, size_t oldsize, int count)
 {
     struct ht_mutable_data_st *newmd = OPENSSL_zalloc(sizeof(*newmd));
     struct ht_mutable_data_st *oldmd = ossl_rcu_deref(&h->md);
@@ -472,7 +478,9 @@ static int grow_hashtable(HT *h, size_t oldsize)
                 /* we ran out of space in a neighborhood, grow again */
                 OPENSSL_free(newmd->neighborhoods);
                 OPENSSL_free(newmd);
-                return grow_hashtable(h, newsize);
+                if (count >= 2)
+                    goto out;
+                return grow_hashtable(h, newsize, ++count);
             }
         }
     }
@@ -525,6 +533,8 @@ static int ossl_ht_insert_locked(HT *h, uint64_t hash,
     uint64_t ihash;
     HT_VALUE *ival;
     size_t empty_idx = SIZE_MAX;
+    struct ht_neighborhood_entry_st check[NEIGHBORHOOD_LEN];
+    struct ht_neighborhood_entry_st check_next[NEIGHBORHOOD_LEN];

     PREFETCH_NEIGHBORHOOD(md->neighborhoods[neigh_idx]);

@@ -539,6 +549,14 @@ static int ossl_ht_insert_locked(HT *h, uint64_t hash,
                 /* invalid */
                 return 0;
             }
+            /* make a copy of the neighborhood */
+            memcpy(check, &md->neighborhoods[neigh_idx].entries,
+                   sizeof(struct ht_neighborhood_entry_st) * NEIGHBORHOOD_LEN);
+            /* also copy the next neighborhood if we're not last in the list */
+            if (neigh_idx != md->neighborhood_mask)
+                memcpy(check_next, &md->neighborhoods[neigh_idx+1].entries,
+                       sizeof(struct ht_neighborhood_entry_st) * NEIGHBORHOOD_LEN);
+
             /* Do a replacement */
             CRYPTO_atomic_store(&md->neighborhoods[neigh_idx].entries[j].hash,
                                 hash, h->atomic_lock);
@@ -546,6 +564,19 @@ static int ossl_ht_insert_locked(HT *h, uint64_t hash,
             ossl_rcu_assign_ptr(&md->neighborhoods[neigh_idx].entries[j].value,
                                 &newval);
             ossl_rcu_call(h->lock, free_old_ht_value, *olddata);
+
+            /* check to make sure that nothing else got changed */
+            if (memcmp(&md->neighborhoods[neigh_idx].entries[j+1], &check[j+1],
+                       (NEIGHBORHOOD_LEN - (j+1))*sizeof(struct ht_neighborhood_entry_st))) {
+                    fprintf(stderr, "FAILED NEIGHBORHOOD VALIDATION ON REPLACE\n");
+                    assert(0);
+            }
+            if (neigh_idx != md->neighborhood_mask &&
+                memcmp(check_next, &md->neighborhoods[neigh_idx+1].entries,
+                       NEIGHBORHOOD_LEN * sizeof(struct ht_neighborhood_entry_st))) {
+                    fprintf(stderr, "FAILED NEXT NEIGHBORHOOD VALIDATION ON REPLACE\n");
+                    assert(0);
+            }
             h->wpd.need_sync = 1;
             return 1;
         }
@@ -554,10 +585,31 @@ static int ossl_ht_insert_locked(HT *h, uint64_t hash,
     if (empty_idx == SIZE_MAX)
         return -1; /* out of space */
     h->wpd.value_count++;
+    /* make a copy of the neighborhood */
+        memcpy(check, &md->neighborhoods[neigh_idx].entries,
+               sizeof(struct ht_neighborhood_entry_st) * NEIGHBORHOOD_LEN);
+    /* also copy the next neighborhood if we're not last in the list */
+    if (neigh_idx != md->neighborhood_mask)
+        memcpy(check_next, &md->neighborhoods[neigh_idx+1].entries,
+               sizeof(struct ht_neighborhood_entry_st) * NEIGHBORHOOD_LEN);
+
     CRYPTO_atomic_store(&md->neighborhoods[neigh_idx].entries[empty_idx].hash,
                         hash, h->atomic_lock);
     ossl_rcu_assign_ptr(&md->neighborhoods[neigh_idx].entries[empty_idx].value,
                         &newval);
+    /* check to make sure that nothing else got changed */
+    if (memcmp(&md->neighborhoods[neigh_idx].entries[empty_idx+1], &check[empty_idx+1],
+           (NEIGHBORHOOD_LEN - (empty_idx+1))*sizeof(struct ht_neighborhood_entry_st))) {
+            fprintf(stderr, "FAILED NEIGHBORHOOD VALIDATION ON INSERT\n");
+            assert(0);
+    }
+    if (neigh_idx != md->neighborhood_mask &&
+        memcmp(check_next, &md->neighborhoods[neigh_idx+1].entries,
+                NEIGHBORHOOD_LEN * sizeof(struct ht_neighborhood_entry_st))) {
+            fprintf(stderr, "FAILED NEXT NEIGHBORHOOD VALIDATION ON INSERT\n");
+            assert(0);
+    }
+
     return 1;
 }

@@ -609,8 +661,9 @@ try_again:
     rc = ossl_ht_insert_locked(h, hash, newval, olddata);

     if (rc == -1) {
-        grow_hashtable(h, h->wpd.neighborhood_len);
-        goto try_again;
+        rc = grow_hashtable(h, h->wpd.neighborhood_len, 0);
+        if (rc != 0)
+            goto try_again;
     }

     if (rc == 0)
@@ -664,6 +717,8 @@ int ossl_ht_delete(HT *h, HT_KEY *key)
     struct ht_internal_value_st *v = NULL;
     HT_VALUE *nv = NULL;
     int rc = 0;
+    struct ht_neighborhood_entry_st check[NEIGHBORHOOD_LEN];
+    struct ht_neighborhood_entry_st check_next[NEIGHBORHOOD_LEN];

     hash = h->config.ht_hash_fn(key->keybuf, key->keysize);

@@ -672,11 +727,31 @@ int ossl_ht_delete(HT *h, HT_KEY *key)
     for (j = 0; j < NEIGHBORHOOD_LEN; j++) {
         if (compare_hash(hash, h->md->neighborhoods[neigh_idx].entries[j].hash)) {
             h->wpd.value_count--;
+            /* make a copy of the neighborhood */
+            memcpy(check, &h->md->neighborhoods[neigh_idx].entries,
+                   sizeof(struct ht_neighborhood_entry_st) * NEIGHBORHOOD_LEN);
+            /* also copy the next neighborhood if we're not last in the list */
+            if (neigh_idx != h->md->neighborhood_mask)
+                memcpy(check_next, &h->md->neighborhoods[neigh_idx+1].entries,
+                       sizeof(struct ht_neighborhood_entry_st) * NEIGHBORHOOD_LEN);
+
             CRYPTO_atomic_store(&h->md->neighborhoods[neigh_idx].entries[j].hash,
                                 0, h->atomic_lock);
             v = (struct ht_internal_value_st *)h->md->neighborhoods[neigh_idx].entries[j].value;
             ossl_rcu_assign_ptr(&h->md->neighborhoods[neigh_idx].entries[j].value,
                                 &nv);
+            /* check to make sure that nothing else got changed */
+            if (memcmp(&h->md->neighborhoods[neigh_idx].entries[j+1], &check[j+1],
+                       (NEIGHBORHOOD_LEN - (j+1))*sizeof(struct ht_neighborhood_entry_st))) {
+                    fprintf(stderr, "FAILED NEIGHBORHOOD VALIDATION ON DELETE\n");
+                    assert(0);
+            }
+            if (neigh_idx != h->md->neighborhood_mask &&
+                memcmp(check_next, &h->md->neighborhoods[neigh_idx+1].entries,
+                       NEIGHBORHOOD_LEN * sizeof(struct ht_neighborhood_entry_st))) {
+                    fprintf(stderr, "FAILED NEXT NEIGHBORHOOD VALIDATION ON DELETE\n");
+                    assert(0);
+            }
             rc = 1;
             break;
         }
diff --git a/crypto/threads_pthread.c b/crypto/threads_pthread.c
index 7b54599947..b114b0d745 100644
--- a/crypto/threads_pthread.c
+++ b/crypto/threads_pthread.c
@@ -354,6 +354,7 @@ struct rcu_lock_st {

     /* signal to wake threads waiting on prior_lock */
     pthread_cond_t prior_signal;
+    int sentry;
 };

 /* Read side acquisition of the current qp */
@@ -568,12 +569,16 @@ static struct rcu_qp *allocate_new_qp_group(CRYPTO_RCU_LOCK *lock,
 void ossl_rcu_write_lock(CRYPTO_RCU_LOCK *lock)
 {
     pthread_mutex_lock(&lock->write_lock);
+    assert(lock->sentry == 0);
+    lock->sentry = 1;
     TSAN_FAKE_UNLOCK(&lock->write_lock);
 }

 void ossl_rcu_write_unlock(CRYPTO_RCU_LOCK *lock)
 {
     TSAN_FAKE_LOCK(&lock->write_lock);
+    assert(lock->sentry == 1);
+    lock->sentry = 0;
     pthread_mutex_unlock(&lock->write_lock);
 }

diff --git a/test/lhash_test.c b/test/lhash_test.c
index 76bed5d7e5..0123d74de2 100644
--- a/test/lhash_test.c
+++ b/test/lhash_test.c
@@ -560,6 +560,8 @@ static void do_mt_hash_work(void)
         switch(behavior) {
         case DO_LOOKUP:
             ossl_ht_read_lock(m_ht);
+            fprintf(stderr, "Doing lookup on entry %d, expected m is %p\n",
+                            index, expected_m);
             m = ossl_ht_mt_TEST_MT_ENTRY_get(m_ht, TO_HT_KEY(&key), &v);
             if (m != NULL && m != expected_m) {
                 worker_exits[num] = "Read unexpected value from hashtable";
@@ -581,6 +583,7 @@ static void do_mt_hash_work(void)
                 r = NULL;
             }

+            fprintf(stderr, "Doing %s on index %d(%p), expected rc is %d\n", behavior == DO_REPLACE ? "Replace" : "Insert", index, expected_m, expected_rc);
             if (expected_rc != ossl_ht_mt_TEST_MT_ENTRY_insert(m_ht,
                                                                TO_HT_KEY(&key),
                                                                expected_m, r)) {
@@ -598,6 +601,8 @@ static void do_mt_hash_work(void)
         case DO_DELETE:
             ossl_ht_write_lock(m_ht);
             expected_rc = expected_m->in_table;
+            fprintf(stderr, "Doing Delete on index %d(%p), expected_rc is %d\n",
+                index, expected_m, expected_rc);
             if (expected_rc != ossl_ht_delete(m_ht, TO_HT_KEY(&key))) {
                 TEST_info("Iteration %d Expected rc %d on delete of element %d which is %s\n",
                           giter, expected_rc, index,

@nhorman working on it, thanks.

This does appear more stable. I have not had a fail or hang in 6 runs.
openssl.lhash.log

I....did not expect that.  But I suppose the next step then would be backing out all the printfs to see if just the recursive growth limitation helps
that would be this patch
diff --git a/crypto/hashtable/hashtable.c b/crypto/hashtable/hashtable.c
index 61d11940a9..362e11ac7a 100644
--- a/crypto/hashtable/hashtable.c
+++ b/crypto/hashtable/hashtable.c
@@ -423,7 +423,7 @@ static void free_old_neigh_table(void *arg)
  * Increase hash table bucket list
  * must be called with write_lock held
  */
-static int grow_hashtable(HT *h, size_t oldsize)
+static int grow_hashtable(HT *h, size_t oldsize, int count)
 {
     struct ht_mutable_data_st *newmd = OPENSSL_zalloc(sizeof(*newmd));
     struct ht_mutable_data_st *oldmd = ossl_rcu_deref(&h->md);
@@ -472,7 +472,9 @@ static int grow_hashtable(HT *h, size_t oldsize)
                 /* we ran out of space in a neighborhood, grow again */
                 OPENSSL_free(newmd->neighborhoods);
                 OPENSSL_free(newmd);
-                return grow_hashtable(h, newsize);
+                if (count >= 2)
+                    goto out;
+                return grow_hashtable(h, newsize, ++count);
             }
         }
     }
@@ -609,8 +611,9 @@ try_again:
     rc = ossl_ht_insert_locked(h, hash, newval, olddata);

     if (rc == -1) {
-        grow_hashtable(h, h->wpd.neighborhood_len);
-        goto try_again;
+        rc = grow_hashtable(h, h->wpd.neighborhood_len, 0);
+        if (rc != 0)
+            goto try_again;
     }

     if (rc == 0)

Question: Is the hash here shared across threads?

Looks like the prints matter, so timing seems to be a factor. Two successive runs, pass and fail:
test-ssl V=1 TESTS=test_lhash
Test started at Thu Jul 18 18:46:08 MDT 2024
"make" depend && "make" _tests
make[1]: Entering directory '/home/ituglib/randall/openssl-klt'
make[1]: Leaving directory '/home/ituglib/randall/openssl-klt'
make[1]: Entering directory '/home/ituglib/randall/openssl-klt'
"make" run_tests
make[2]: Entering directory '/home/ituglib/randall/openssl-klt'
( SRCTOP=. \
  BLDTOP=. \
  PERL="perl" \
  FIPSKEY="f4556650ac31d35461610bac4ed81b1a181b2d8a43ea2854cbae22ca74560813" \
  EXE_EXT= \
  perl ./test/run_tests.pl test_lhash )
00-prep_fipsmodule_cnf.t ..

Files=1, Tests=1,  2 wallclock secs ( 0.01 usr  0.00 sys +  0.97 cusr  0.00 csys =  0.98 CPU)
Result: PASS
02-test_lhash.t ..
# The results of this test will end up in test-runs/test_lhash
1..1
    # Subtest: ../../test/lhash_test
    1..5
    ok 1 - test_int_lhash
    ok 2 - test_stress
    ok 3 - test_int_hashtable
    ok 4 - test_hashtable_stress
    ok 5 - test_hashtable_multithread
../../util/wrap.pl ../../test/lhash_test => 0
ok 1 - running lhash_test
ok
All tests successful.
Files=1, Tests=1,  3 wallclock secs ( 0.00 usr  0.00 sys +  3.40 cusr  0.00 csys =  3.41 CPU)
Result: PASS
make[2]: Leaving directory '/home/ituglib/randall/openssl-klt'
make[1]: Leaving directory '/home/ituglib/randall/openssl-klt'
Test ended at Thu Jul 18 18:46:14 MDT 2024
/home/ituglib/randall/openssl-klt: test-ssl V=1 TESTS=test_lhash
Test started at Thu Jul 18 18:46:17 MDT 2024
"make" depend && "make" _tests
make[1]: Entering directory '/home/ituglib/randall/openssl-klt'
make[1]: Leaving directory '/home/ituglib/randall/openssl-klt'
make[1]: Entering directory '/home/ituglib/randall/openssl-klt'
"make" run_tests
make[2]: Entering directory '/home/ituglib/randall/openssl-klt'
( SRCTOP=. \
  BLDTOP=. \
  PERL="perl" \
  FIPSKEY="f4556650ac31d35461610bac4ed81b1a181b2d8a43ea2854cbae22ca74560813" \
  EXE_EXT= \
  perl ./test/run_tests.pl test_lhash )
00-prep_fipsmodule_cnf.t ..

Files=1, Tests=1,  2 wallclock secs ( 0.01 usr  0.00 sys +  0.97 cusr  0.00 csys =  0.98 CPU)
Result: PASS
02-test_lhash.t ..
# The results of this test will end up in test-runs/test_lhash
1..1
    # Subtest: ../../test/lhash_test
    1..5
    ok 1 - test_int_lhash
    ok 2 - test_stress
    ok 3 - test_int_hashtable
    ok 4 - test_hashtable_stress
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 18286 Expected rc 0 on insert of element 97 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 18293 Expected rc 0 on insert of element 55 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 18299 Expected rc 0 on insert of element 195 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 18301 Expected rc 0 on insert of element 212 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 0 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 1 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 2 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 3 failed: Failure on insert
    #
    # OPENSSL_TEST_RAND_SEED=1721328380
    not ok 5 - test_hashtable_multithread
../../util/wrap.pl ../../test/lhash_test => 255
not ok 1 - running lhash_test

#   Failed test 'running lhash_test'
#   at /home/ituglib/randall/openssl-klt/util/perl/OpenSSL/Test/Simple.pm line 77.
# Looks like you failed 1 test of 1.
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests

Test Summary Report
-------------------
02-test_lhash.t (Wstat: 256 Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1
Files=1, Tests=1,  3 wallclock secs ( 0.00 usr  0.00 sys +  3.38 cusr  0.00 csys =  3.39 CPU)
Result: FAIL
Makefile:1766: recipe for target 'run_tests' failed
make[2]: *** [run_tests] Error 1
make[2]: Leaving directory '/home/ituglib/randall/openssl-klt'
Makefile:1764: recipe for target '_tests' failed
make[1]: *** [_tests] Error 2
make[1]: Leaving directory '/home/ituglib/randall/openssl-klt'
Makefile:1762: recipe for target 'tests' failed
make: *** [tests] Error 2
Test ended at Thu Jul 18 18:46:23 MDT 2024

Regarding your question about hashes, the hash function is common and works like any hash table.  A given unique key value consistently maps to a consistent unit64_t hash value

@nhorman Are the inserts/deletes thread-safe?

Ok, so I should add in some checking without the printfs to keep timing similar

Yes, the inserts/deletes in this test are done under the protection of a pthread_mutex_lock

Just out of curiosity, on this same platform, does this command pass?
make TESTS=test_threads test

Just out of curiosity, on this same platform, does this command pass?
make TESTS=test_threads test

Yes - passing. It was failing under build 1 of the new threading model (a lot of complaining and groveling on my part), but has worked since 2 and now on 3.

I've been rerunning test_threads... no problems found.

ok, so we can be reasonably confident that thread mutexes are working as expected.
Heres a patch to re-introduce some of the memory checking, with less overhead/printfs to keep the timing similar.
diff --git a/crypto/hashtable/hashtable.c b/crypto/hashtable/hashtable.c
index 61d11940a9..79f684490b 100644
--- a/crypto/hashtable/hashtable.c
+++ b/crypto/hashtable/hashtable.c
@@ -47,6 +47,7 @@
  */

 #include <string.h>
+#include <assert.h>
 #include <internal/rcu.h>
 #include <internal/hashtable.h>
 #include <openssl/rand.h>
@@ -423,7 +424,7 @@ static void free_old_neigh_table(void *arg)
  * Increase hash table bucket list
  * must be called with write_lock held
  */
-static int grow_hashtable(HT *h, size_t oldsize)
+static int grow_hashtable(HT *h, size_t oldsize, int count)
 {
     struct ht_mutable_data_st *newmd = OPENSSL_zalloc(sizeof(*newmd));
     struct ht_mutable_data_st *oldmd = ossl_rcu_deref(&h->md);
@@ -472,7 +473,11 @@ static int grow_hashtable(HT *h, size_t oldsize)
                 /* we ran out of space in a neighborhood, grow again */
                 OPENSSL_free(newmd->neighborhoods);
                 OPENSSL_free(newmd);
-                return grow_hashtable(h, newsize);
+                if (count >= 2) {
+                    rc = -1;
+                    goto out;
+                }
+                return grow_hashtable(h, newsize, ++count);
             }
         }
     }
@@ -525,6 +530,7 @@ static int ossl_ht_insert_locked(HT *h, uint64_t hash,
     uint64_t ihash;
     HT_VALUE *ival;
     size_t empty_idx = SIZE_MAX;
+    struct ht_neighborhood_entry_st check;

     PREFETCH_NEIGHBORHOOD(md->neighborhoods[neigh_idx]);

@@ -539,13 +545,32 @@ static int ossl_ht_insert_locked(HT *h, uint64_t hash,
                 /* invalid */
                 return 0;
             }
+            if (j < (NEIGHBORHOOD_LEN-1)) {
+                memcpy(&check, &md->neighborhoods[neigh_idx].entries[j+1], sizeof(struct ht_neighborhood_entry_st));
+            } else if (neigh_idx != md->neighborhood_mask) {
+                memcpy(&check, &md->neighborhoods[neigh_idx+1].entries[0], sizeof(struct ht_neighborhood_entry_st));
+            }
             /* Do a replacement */
             CRYPTO_atomic_store(&md->neighborhoods[neigh_idx].entries[j].hash,
                                 hash, h->atomic_lock);
             *olddata = (HT_VALUE *)md->neighborhoods[neigh_idx].entries[j].value;
             ossl_rcu_assign_ptr(&md->neighborhoods[neigh_idx].entries[j].value,
                                 &newval);
+
+            if (j < (NEIGHBORHOOD_LEN-1)) {
+                if (memcmp(&check, &md->neighborhoods[neigh_idx].entries[j+1], sizeof(struct ht_neighborhood_entry_st))) {
+                    fprintf(stderr, "FAILED TO VALIDATE NEXT J ON REPLACE\n");
+                    assert(0);
+                }
+            } else if (neigh_idx != md->neighborhood_mask) {
+                if (memcmp(&check, &md->neighborhoods[neigh_idx+1].entries[0], sizeof(struct ht_neighborhood_entry_st))) {
+                    fprintf(stderr, "FAILED TO VALIDATE NEXT NEIGH ON REPLACE\n");
+                    assert(0);
+                }
+            }
+
             ossl_rcu_call(h->lock, free_old_ht_value, *olddata);
+
             h->wpd.need_sync = 1;
             return 1;
         }
@@ -554,10 +579,28 @@ static int ossl_ht_insert_locked(HT *h, uint64_t hash,
     if (empty_idx == SIZE_MAX)
         return -1; /* out of space */
     h->wpd.value_count++;
+    if (empty_idx < (NEIGHBORHOOD_LEN-1)) {
+            memcpy(&check, &md->neighborhoods[neigh_idx].entries[empty_idx+1], sizeof(struct ht_neighborhood_entry_st));
+    } else if (neigh_idx != md->neighborhood_mask) {
+            memcpy(&check, &md->neighborhoods[neigh_idx+1].entries[0], sizeof(struct ht_neighborhood_entry_st));
+    }
+
     CRYPTO_atomic_store(&md->neighborhoods[neigh_idx].entries[empty_idx].hash,
                         hash, h->atomic_lock);
     ossl_rcu_assign_ptr(&md->neighborhoods[neigh_idx].entries[empty_idx].value,
                         &newval);
+
+    if (empty_idx < (NEIGHBORHOOD_LEN-1)) {
+        if (memcmp(&check, &md->neighborhoods[neigh_idx].entries[empty_idx+1], sizeof(struct ht_neighborhood_entry_st))) {
+            fprintf(stderr, "FAILED TO VALIDATE NEXT J ON INSERT\n");
+            assert(0);
+        }
+    } else if (neigh_idx != md->neighborhood_mask) {
+        if (memcmp(&check, &md->neighborhoods[neigh_idx+1].entries[0], sizeof(struct ht_neighborhood_entry_st))) {
+            fprintf(stderr, "FAILED TO VALIDATE NEXT NEIGH ON INSERT\n");
+            assert(0);
+        }
+    }
     return 1;
 }

@@ -609,8 +652,9 @@ try_again:
     rc = ossl_ht_insert_locked(h, hash, newval, olddata);

     if (rc == -1) {
-        grow_hashtable(h, h->wpd.neighborhood_len);
-        goto try_again;
+        rc = grow_hashtable(h, h->wpd.neighborhood_len, 0);
+        if (rc == 1)
+            goto try_again;
     }

     if (rc == 0)
@@ -664,6 +708,7 @@ int ossl_ht_delete(HT *h, HT_KEY *key)
     struct ht_internal_value_st *v = NULL;
     HT_VALUE *nv = NULL;
     int rc = 0;
+    struct ht_neighborhood_entry_st check;

     hash = h->config.ht_hash_fn(key->keybuf, key->keysize);

@@ -672,11 +717,28 @@ int ossl_ht_delete(HT *h, HT_KEY *key)
     for (j = 0; j < NEIGHBORHOOD_LEN; j++) {
         if (compare_hash(hash, h->md->neighborhoods[neigh_idx].entries[j].hash)) {
             h->wpd.value_count--;
+           if (j < (NEIGHBORHOOD_LEN-1)) {
+                memcpy(&check, &h->md->neighborhoods[neigh_idx].entries[j+1], sizeof(struct ht_neighborhood_entry_st));
+           } else if (neigh_idx != h->md->neighborhood_mask) {
+                memcpy(&check, &h->md->neighborhoods[neigh_idx+1].entries[0], sizeof(struct ht_neighborhood_entry_st));
+           }
             CRYPTO_atomic_store(&h->md->neighborhoods[neigh_idx].entries[j].hash,
                                 0, h->atomic_lock);
             v = (struct ht_internal_value_st *)h->md->neighborhoods[neigh_idx].entries[j].value;
             ossl_rcu_assign_ptr(&h->md->neighborhoods[neigh_idx].entries[j].value,
                                 &nv);
+
+            if (j < (NEIGHBORHOOD_LEN-1)) {
+                if (memcmp(&check, &h->md->neighborhoods[neigh_idx].entries[j+1], sizeof(struct ht_neighborhood_entry_st))) {
+                    fprintf(stderr, "FAILED TO VALIDATE NEXT J ON INSERT\n");
+                    assert(0);
+                }
+            } else if (neigh_idx != h->md->neighborhood_mask) {
+                if (memcmp(&check, &h->md->neighborhoods[neigh_idx+1].entries[0], sizeof(struct ht_neighborhood_entry_st))) {
+                    fprintf(stderr, "FAILED TO VALIDATE NEXT NEIGH ON INSERT\n");
+                    assert(0);
+                }
+            }
             rc = 1;
             break;
         }

Please configure with:
./Configure -d
when building, as that will trigger assertion failures

So far, 3 for 3 PASS with those changes.

I put in an extra assert here:
            if (olddata == NULL) {
                /* invalid */
                assert(0);
                return 0;
            }

because it said "Invalid", I figured it was. This assert was hit each test run. Not sure this matters, but it was.

Failures (5 out of 10) finally showed after removing the above assert:
02-test_lhash.t ..
# The results of this test will end up in test-runs/test_lhash
1..1
    # Subtest: ../../test/lhash_test
    1..5
    ok 1 - test_int_lhash
    ok 2 - test_stress
    ok 3 - test_int_hashtable
    ok 4 - test_hashtable_stress
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 8470 Expected rc 0 on insert of element 26 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 8477 Expected rc 0 on insert of element 45 which is in table
    #
\NS8NG8.$:0:849:145527660 - *** Run-time Error 034 ***
\NS8NG8.$:0:849:145527660 - Released space not allocated
../../util/wrap.pl ../../test/lhash_test => 159

02-test_lhash.t ..
# The results of this test will end up in test-runs/test_lhash
1..1
    # Subtest: ../../test/lhash_test
    1..5
    ok 1 - test_int_lhash
    ok 2 - test_stress
    ok 3 - test_int_hashtable
    ok 4 - test_hashtable_stress
\NS8NG8.$:0:885:145535852 - *** Run-time Error 034 ***
\NS8NG8.$:0:885:145535852 - Released space not allocated
../../util/wrap.pl ../../test/lhash_test => 159
not ok 1 - running lhash_test

#   Failed test 'running lhash_test'
#   at /home/ituglib/randall/openssl-klt/util/perl/OpenSSL/Test/Simple.pm line 77.
# Looks like you failed 1 test of 1.
Dubious, test returned 1 (wstat 256, 0x100)

02-test_lhash.t ..
# The results of this test will end up in test-runs/test_lhash
1..1
    # Subtest: ../../test/lhash_test
    1..5
    ok 1 - test_int_lhash
    ok 2 - test_stress
    ok 3 - test_int_hashtable
    ok 4 - test_hashtable_stress
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 17323 Expected rc 0 on insert of element 190 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 17327 Expected rc 0 on insert of element 163 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 17329 Expected rc 0 on insert of element 131 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 17331 Expected rc 0 on insert of element 159 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 0 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 1 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 2 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 3 failed: Failure on insert
    #
    # OPENSSL_TEST_RAND_SEED=1721339280
    not ok 5 - test_hashtable_multithread
../../util/wrap.pl ../../test/lhash_test => 255
not ok 1 - running lhash_test

02-test_lhash.t ..
# The results of this test will end up in test-runs/test_lhash
1..1
    # Subtest: ../../test/lhash_test
    1..5
    ok 1 - test_int_lhash
    ok 2 - test_stress
    ok 3 - test_int_hashtable
    ok 4 - test_hashtable_stress
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 6841 Expected rc 0 on insert of element 215 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 6845 Expected rc 0 on insert of element 171 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 6855 Expected rc 0 on insert of element 143 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 6860 Expected rc 0 on insert of element 74 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 0 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 1 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 2 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 3 failed: Failure on insert
    #
    # OPENSSL_TEST_RAND_SEED=1721339292
    not ok 5 - test_hashtable_multithread
../../util/wrap.pl ../../test/lhash_test => 255
not ok 1 - running lhash_test

#   Failed test 'running lhash_test'
#   at /home/ituglib/randall/openssl-klt/util/perl/OpenSSL/Test/Simple.pm line 77.
# Looks like you failed 1 test of 1.

02-test_lhash.t ..
# The results of this test will end up in test-runs/test_lhash
1..1
    # Subtest: ../../test/lhash_test
    1..5
    ok 1 - test_int_lhash
    ok 2 - test_stress
    ok 3 - test_int_hashtable
    ok 4 - test_hashtable_stress
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 615 Expected rc 0 on insert of element 74 which is in table
    #
\NS8NG8.$:0:458:145593196 - *** Run-time Error 034 ***
\NS8NG8.$:0:458:145593196 - Released space not allocated
../../util/wrap.pl ../../test/lhash_test => 159
not ok 1 - running lhash_test

#   Failed test 'running lhash_test'
#   at /home/ituglib/randall/openssl-klt/util/perl/OpenSSL/Test/Simple.pm line 77.
# Looks like you failed 1 test of 1.
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests

@nhorman FYI: When the stack trace is:
\NS8NG8.$:0:458:145593196 - *** Run-time Error 034 ***
\NS8NG8.$:0:458:145593196 - Released space not allocated

This is seems to be happening near or around the main method of a thread, on the current OS build.

You should remove the assert from:
if (olddata == NULL) {
                /* invalid */
                return 0;
            }

That is checking for the condition in which we are doing an insert but have a conflicting entry in the hash table.  i.e. it is invalid to do have a colliding hash entry if you're not doing a replacement op.  The result is that we should fail the insert.  May be invalid isn't  a great name there

Already removed. The above results are with that assertion gone.

Can you add this printf to the top of ossl_ht_free()?
fprintf(stderr, "FREEING NEIGHTBORHOOD %p\n", h->md->neighborhood_ptr_to_free);

Can you add this printf to the top of ossl_ht_free()?
fprintf(stderr, "FREEING NEIGHTBORHOOD %p\n", h->md->neighborhood_ptr_to_free);

Working on it. Will report results.

Failed tests (out of 10 runs - one passes for reference):
1..1
    # Subtest: ../../test/lhash_test
    1..5
    ok 1 - test_int_lhash
    ok 2 - test_stress
FREEING NEIGHTBORHOOD 100024ea0
    ok 3 - test_int_hashtable
FREEING NEIGHTBORHOOD 100024ea0
    ok 4 - test_hashtable_stress
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 1882 Expected rc 0 on insert of element 32 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 1885 Expected rc 0 on insert of element 52 which is in table
    #
\NS8NG8.$:0:554:149085804 - *** Run-time Error 034 ***
\NS8NG8.$:0:554:149085804 - Released space not allocated
../../util/wrap.pl ../../test/lhash_test => 159
not ok 1 - running lhash_test

#   Failed test 'running lhash_test'
#   at /home/ituglib/randall/openssl-klt/util/perl/OpenSSL/Test/Simple.pm line 77.
# Looks like you failed 1 test of 1.
Dubious, test returned 1 (wstat 256, 0x100)

1..1
    # Subtest: ../../test/lhash_test
    1..5
    ok 1 - test_int_lhash
    ok 2 - test_stress
FREEING NEIGHTBORHOOD 100024ea0
    ok 3 - test_int_hashtable
FREEING NEIGHTBORHOOD 100024ea0
    ok 4 - test_hashtable_stress
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 223 Expected rc 0 on insert of element 168 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 224 Expected rc 0 on insert of element 234 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 226 Expected rc 0 on insert of element 228 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 256 Expected rc 0 on insert of element 34 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 0 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 1 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 2 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 3 failed: Failure on insert
    #
FREEING NEIGHTBORHOOD 1000755a0
    # OPENSSL_TEST_RAND_SEED=1721492603
    not ok 5 - test_hashtable_multithread
../../util/wrap.pl ../../test/lhash_test => 255

1..1
    # Subtest: ../../test/lhash_test
    1..5
    ok 1 - test_int_lhash
    ok 2 - test_stress
FREEING NEIGHTBORHOOD 100024ea0
    ok 3 - test_int_hashtable
FREEING NEIGHTBORHOOD 100024ea0
    ok 4 - test_hashtable_stress
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 17400 Expected rc 0 on insert of element 109 which is in table
    #
\NS8NG8.$:0:664:149102188 - *** Run-time Error 034 ***
\NS8NG8.$:0:664:149102188 - Released space not allocated
../../util/wrap.pl ../../test/lhash_test => 159

1..1
    # Subtest: ../../test/lhash_test
    1..5
    ok 1 - test_int_lhash
    ok 2 - test_stress
FREEING NEIGHTBORHOOD 100024ea0
    ok 3 - test_int_hashtable
FREEING NEIGHTBORHOOD 100024ea0
    ok 4 - test_hashtable_stress
FREEING NEIGHTBORHOOD 100075370
    ok 5 - test_hashtable_multithread
../../util/wrap.pl ../../test/lhash_test => 0
ok 1 - running lhash_test

1..1
    # Subtest: ../../test/lhash_test
    1..5
    ok 1 - test_int_lhash
    ok 2 - test_stress
FREEING NEIGHTBORHOOD 100024ea0
    ok 3 - test_int_hashtable
FREEING NEIGHTBORHOOD 100024ea0
    ok 4 - test_hashtable_stress
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 7265 Expected rc 0 on insert of element 57 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 7268 Expected rc 0 on insert of element 204 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 7270 Expected rc 0 on insert of element 68 which is in table
    #
\NS8NG8.$:0:765:149118572 - *** Run-time Error 034 ***
\NS8NG8.$:0:765:149118572 - Released space not allocated
../../util/wrap.pl ../../test/lhash_test => 159

1..1
    # Subtest: ../../test/lhash_test
    1..5
    ok 1 - test_int_lhash
    ok 2 - test_stress
FREEING NEIGHTBORHOOD 100024ea0
    ok 3 - test_int_hashtable
FREEING NEIGHTBORHOOD 100024ea0
    ok 4 - test_hashtable_stress
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 26605 Expected rc 0 on insert of element 58 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 26612 Expected rc 0 on insert of element 185 which is in table
    #
\NS8NG8.$:0:822:149134956 - *** Run-time Error 034 ***
\NS8NG8.$:0:822:149134956 - Released space not allocated
../../util/wrap.pl ../../test/lhash_test => 159

1..1
    # Subtest: ../../test/lhash_test
    1..5
    ok 1 - test_int_lhash
    ok 2 - test_stress
FREEING NEIGHTBORHOOD 100024ea0
    ok 3 - test_int_hashtable
FREEING NEIGHTBORHOOD 100024ea0
    ok 4 - test_hashtable_stress
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 26178 Expected rc 0 on insert of element 112 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 26181 Expected rc 0 on insert of element 104 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 26185 Expected rc 0 on insert of element 105 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:589
    # Iteration 26186 Expected rc 0 on insert of element 178 which is in table
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 0 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 1 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 2 failed: Failure on insert
    #
    # INFO:  @ /home/ituglib/randall/openssl-klt/test/lhash_test.c:671
    # Worker 3 failed: Failure on insert
    #
FREEING NEIGHTBORHOOD 100075730
    # OPENSSL_TEST_RAND_SEED=1721492637
    not ok 5 - test_hashtable_multithread
../../util/wrap.pl ../../test/lhash_test => 255

so, I'm at something of a loss here.  I'm not at all sure whats going on.  the next thing I can think to do is run this patch:
diff --git a/crypto/hashtable/hashtable.c b/crypto/hashtable/hashtable.c
index 61d11940a9..97778fb8eb 100644
--- a/crypto/hashtable/hashtable.c
+++ b/crypto/hashtable/hashtable.c
@@ -688,3 +688,24 @@ int ossl_ht_delete(HT *h, HT_KEY *key)
     return rc;
 }

+void ossl_ht_dump(HT *h, void (*dump_cb)(HT_VALUE *val))
+{
+    size_t i, j;
+
+    for (i = 0; i < h->md->neighborhood_mask; i++) {
+        fprintf(stderr, "NEIGHBORHOOD %lu:\n", i);
+        for (j = 0; j < NEIGHBORHOOD_LEN; j++) {
+            fprintf(stderr, "\tNEIGHBORHOOD ENTRY:%lu HASH: %lu VALUE PTR: %p\n\t\t", j, h->md->neighborhoods[i].entries[j].hash, h->md->neighborhoods[i].entries[j].value);
+            if (h->md->neighborhoods[i].entries[j].value)
+                dump_cb((HT_VALUE *)h->md->neighborhoods[i].entries[j].value);
+            else
+                fprintf(stderr, "NO VALUE\n");
+        }
+    }
+}
+
+
+uint64_t ossl_ht_hash(HT *h, HT_KEY *key)
+{
+    return h->config.ht_hash_fn(key->keybuf, key->keysize);
+}
diff --git a/include/internal/hashtable.h b/include/internal/hashtable.h
index 873612d827..c907926dea 100644
--- a/include/internal/hashtable.h
+++ b/include/internal/hashtable.h
@@ -329,4 +329,6 @@ void ossl_ht_value_list_free(HT_VALUE_LIST *list);
  */
 HT_VALUE *ossl_ht_get(HT *htable, HT_KEY *key);

+void ossl_ht_dump(HT *htable, void (*dump_cb)(HT_VALUE *val));
+uint64_t ossl_ht_hash(HT *h, HT_KEY *key);
 #endif
diff --git a/test/lhash_test.c b/test/lhash_test.c
index 76bed5d7e5..51d5b2e42c 100644
--- a/test/lhash_test.c
+++ b/test/lhash_test.c
@@ -470,6 +470,7 @@ end:
 typedef struct test_mt_entry {
     int in_table;
     int pending_delete;
+    uint64_t hash;
 } TEST_MT_ENTRY;

 static HT *m_ht = NULL;
@@ -619,6 +620,13 @@ static void do_mt_hash_work(void)
     }
 }

+static void print_value(HT_VALUE *v)
+{
+    TEST_MT_ENTRY *m = ossl_ht_mt_TEST_MT_ENTRY_from_value(v);
+
+    fprintf(stderr, "ptr:%p, in_table:%d, pending_delete:%d hash: %lu\n", m, m->in_table, m->pending_delete, m->hash);
+}
+
 static int test_hashtable_multithread(void)
 {
     HT_CONFIG hash_conf = {
@@ -630,6 +638,8 @@ static int test_hashtable_multithread(void)
     int ret = 0;
     thread_t workers[4];
     int i;
+    MTKEY key;
+
 #ifdef MEASURE_HASH_PERFORMANCE
     struct timeval start, end, delta;
 #endif
@@ -643,6 +653,13 @@ static int test_hashtable_multithread(void)
     if (!TEST_ptr(m_ht))
         goto end;

+    HT_INIT_KEY(&key);
+    for (i = 0; i < TEST_MT_POOL_SZ; i++) {
+        HT_KEY_RESET(&key);
+        HT_SET_KEY_FIELD(&key, index, i);
+        test_mt_entries[i].hash = ossl_ht_hash(m_ht, TO_HT_KEY(&key));
+    }
+
     worker_lock = CRYPTO_THREAD_lock_new();
     if (worker_lock == NULL)
         goto end_free;
@@ -685,6 +702,7 @@ shutdown:

 end_free:
     shutting_down = 1;
+    ossl_ht_dump(m_ht, print_value);
     ossl_ht_free(m_ht);
 end:
     return ret;

This will add the expected hash for each entry in the hash table to each test table entry, and then dump the entire hash table at the end of the multithread run.  We can use this to attempt to identify any inconsistencies between the expectation table and the actual hash table.  We can use that to identify next steps.

@nhorman Thanks. Will apply this and report results.

Results are mixed. I've attached the log. Hopefully @nhorman can see something.
openssl.lhash.log

Thank you, some progress!
This immediately looks out of place:
NEIGHBORHOOD ENTRY:0 HASH:<PHONE_NUMBER>41568906 VALUE PTR: 100075860
ptr:80d64a0, in_table:1, pending_delete:0 hash:<PHONE_NUMBER>41568906
NEIGHBORHOOD ENTRY:1 HASH: 0 VALUE PTR: 0
NO VALUE
NEIGHBORHOOD ENTRY:2 HASH: 0 VALUE PTR: 0
NO VALUE
NEIGHBORHOOD ENTRY:3 HASH: 0 VALUE PTR: 100011590
ptr:80d64a0, in_table:1, pending_delete:0 hash:<PHONE_NUMBER>41568906

It appears we have two entries in a neighborhood for the same data, which is bad.
Entry 3 appears to be in the process of getting deleted (its HASH value is 0, but the value hasn't been removed yet
I'm not sure how that might have occured, as entries and deletes are supposed to be serialized under the hash table lock.  Let me see if I can figure out how that might have happened.

Two value pointers hashing to the same value. Should we be printing the actual value? I'm wondering about the math - being a Big Endian system but on x86_64 done in millicode.

Trying. I have kept all of the patches, so this might be noisier. -d is enabled.

In answer to your question above @rsbeckerca this hashtable works differently than the origional hash table we have.
this table separates keys from values.  each key is defined as follows:
struct key {
    struct key_header {
         size_t keylen;
         uint8_t *keybuf;
     }
     struct {
           ... Whatever key fields you want to define....
     } keyfields;
};

when HT_KEY_INIT is called, the key header fields are initalized, with keylen being the size of the key being the size of the keyfields struct and keybuf pointing to the start of the keyfields struct member.  When we insert or delete, the passed key is is treated as a unit8_t buffer of length keylen, and hashed, starting at keybuf using the fnv1a_hash function.
It works consistently on our supported platforms, and appears to work (generally speaking) on non-stop.  but we seem to have what would otherwise be a valid entry in the hash table, save for the fact that its hash value is zero.
The odd part is, if you look at the table dump here
the HASH: printf is the value that the hashtable has stored for that element while the hash: value is the precomputed hash value for that element in the static table that the test defines.
The fact that:
NEIGHBORHOOD ENTRY:0 HASH:<PHONE_NUMBER>41568906 VALUE PTR: 100075860
ptr:80d64a0, in_table:1, pending_delete:0 hash:<PHONE_NUMBER>41568906

shows the same hash value in the hash table and the static data table suggests to me that we 're getting consistent results, which is good, but this:
NEIGHBORHOOD ENTRY:3 HASH: 0 VALUE PTR: 100011590
ptr:80d64a0, in_table:1, pending_delete:0 hash:<PHONE_NUMBER>41568906

suggests that something has gone wrong in its computation somewhere
another data point:  This is neighborhood 10 in the hash table.  Neighborhoods are identified by a neighborhood mask value.  Their like buckets.  That is to say a hash value of 0x0123456789abcd0a would map to neighborhood 10, because in a table of 64 neighborhoods, the mask should be 0x3f, and (0x0123456789abcd0a & 0x3f) == 0x0a
So at some point when this entry was added, the hash value was coherent enough to place this in neighborhood 10, but between the time that neighborhood index was computed, the hash value went from whatever value it was, to zero.  Thats super wierd, since the hash value is on the stack when this occurs, so unless we're seeing some stack smashing, that shouldn't happen.
An alternate theory would be that, on delete, the hash value for this entry is getting reset to zero properly, but the corresponding value entry isn't getting set to NULL, can't see a way for that to happen at the moment, but there is a test in the above patch to catch that now.

hey, I just saw something, but I want to confirm something with you first.  Non-stop when compiling thread_pthread.c, does it select that __atomic_store operations when you build, or does it fall back to  using pthread locks?
this is currently the definition of CRYPTO_atomic_store, which the hashtable uses to assign hash values:
int CRYPTO_atomic_store(uint64_t *dst, uint64_t val, CRYPTO_RWLOCK *lock)
{
# if defined(__GNUC__) && defined(__ATOMIC_ACQUIRE) && !defined(BROKEN_CLANG_ATOMICS)
    if (__atomic_is_lock_free(sizeof(*dst), dst)) {
        __atomic_store(dst, &val, __ATOMIC_RELEASE);
        return 1;
    }
# elif defined(__sun) && (defined(__SunOS_5_10) || defined(__SunOS_5_11))
    /* This will work for all future Solaris versions. */
    if (ret != NULL) {
        atomic_swap_64(dst, val);
        return 1;
    }
# endif
    if (lock == NULL || !CRYPTO_THREAD_read_lock(lock))
        return 0;
    *dst  = val;
    if (!CRYPTO_THREAD_unlock(lock))
        return 0;

    return 1;
}

i just noticed that the lock in that last part uses a read lock rather than a write lock, and it should definitely be the latter.  I don't immediately see how it would matter here, as we should already be protected by the hashtable write side mutex for all insert/delete operations, but it definitely stands out.  So the question is does your compiler/libc define GNUC and __ATOMIC_ACQUIRE?

just in case heres an ammended patch to fix that:
diff --git a/crypto/hashtable/hashtable.c b/crypto/hashtable/hashtable.c
index 61d11940a9..7fc8369421 100644
--- a/crypto/hashtable/hashtable.c
+++ b/crypto/hashtable/hashtable.c
@@ -47,6 +47,7 @@
  */

 #include <string.h>
+#include <assert.h>
 #include <internal/rcu.h>
 #include <internal/hashtable.h>
 #include <openssl/rand.h>
@@ -591,6 +592,7 @@ int ossl_ht_insert(HT *h, HT_KEY *key, HT_VALUE *data, HT_VALUE **olddata)
     struct ht_internal_value_st *newval = NULL;
     uint64_t hash;
     int rc = 0;
+    int i;

     if (data->value == NULL)
         goto out;
@@ -605,6 +607,15 @@ int ossl_ht_insert(HT *h, HT_KEY *key, HT_VALUE *data, HT_VALUE **olddata)
      */
     hash = h->config.ht_hash_fn(key->keybuf, key->keysize);

+    if (hash == 0) {
+        fprintf(stderr, "HASH EVALUATED TO 0 on key:!\n");
+        fprintf(stderr, "KEYSIZE %lu, KEYBUF %p\n", key->keysize, key->keybuf);
+        for (i=0; i < key->keysize; i++) {
+            fprintf(stderr, "%02x", key->keybuf[i]);
+        }
+        fprintf(stderr, "\n");
+        assert(0);
+    }
 try_again:
     rc = ossl_ht_insert_locked(h, hash, newval, olddata);

@@ -661,12 +672,23 @@ int ossl_ht_delete(HT *h, HT_KEY *key)
     uint64_t hash;
     uint64_t neigh_idx;
     size_t j;
+    int i;
     struct ht_internal_value_st *v = NULL;
     HT_VALUE *nv = NULL;
     int rc = 0;

     hash = h->config.ht_hash_fn(key->keybuf, key->keysize);

+    if (hash == 0) {
+        fprintf(stderr, "HASH EVALUATED TO 0 on key:!\n");
+        fprintf(stderr, "KEYSIZE %lu, KEYBUF %p\n", key->keysize, key->keybuf);
+        for (i=0; i < key->keysize; i++) {
+            fprintf(stderr, "%02x", key->keybuf[i]);
+        }
+        fprintf(stderr, "\n");
+        assert(0);
+    }
+
     neigh_idx = hash & h->md->neighborhood_mask;
     PREFETCH_NEIGHBORHOOD(h->md->neighborhoods[neigh_idx]);
     for (j = 0; j < NEIGHBORHOOD_LEN; j++) {
@@ -677,6 +699,11 @@ int ossl_ht_delete(HT *h, HT_KEY *key)
             v = (struct ht_internal_value_st *)h->md->neighborhoods[neigh_idx].entries[j].value;
             ossl_rcu_assign_ptr(&h->md->neighborhoods[neigh_idx].entries[j].value,
                                 &nv);
+            /* confirm that the value is NULL */
+            if (h->md->neighborhoods[neigh_idx].entries[j].value != NULL) {
+                fprintf(stderr, "DELETED ENTRY IS NOT NULL\n");
+                assert(0);
+            }
             rc = 1;
             break;
         }
@@ -688,3 +715,24 @@ int ossl_ht_delete(HT *h, HT_KEY *key)
     return rc;
 }

+void ossl_ht_dump(HT *h, void (*dump_cb)(HT_VALUE *val))
+{
+    size_t i, j;
+
+    for (i = 0; i <= h->md->neighborhood_mask; i++) {
+        fprintf(stderr, "NEIGHBORHOOD %lu:\n", i);
+        for (j = 0; j < NEIGHBORHOOD_LEN; j++) {
+            fprintf(stderr, "\tNEIGHBORHOOD ENTRY:%lu HASH: %lu VALUE PTR: %p\n\t\t", j, h->md->neighborhoods[i].entries[j].hash, h->md->neighborhoods[i].entries[j].value);
+            if (h->md->neighborhoods[i].entries[j].value)
+                dump_cb((HT_VALUE *)h->md->neighborhoods[i].entries[j].value);
+            else
+                fprintf(stderr, "NO VALUE\n");
+        }
+    }
+}
+
+
+uint64_t ossl_ht_hash(HT *h, HT_KEY *key)
+{
+    return h->config.ht_hash_fn(key->keybuf, key->keysize);
+}
diff --git a/crypto/threads_pthread.c b/crypto/threads_pthread.c
index 14b1dbf6c2..86943ba433 100644
--- a/crypto/threads_pthread.c
+++ b/crypto/threads_pthread.c
@@ -994,7 +994,7 @@ int CRYPTO_atomic_store(uint64_t *dst, uint64_t val, CRYPTO_RWLOCK *lock)
         return 1;
     }
 # endif
-    if (lock == NULL || !CRYPTO_THREAD_read_lock(lock))
+    if (lock == NULL || !CRYPTO_THREAD_write_lock(lock))
         return 0;
     *dst  = val;
     if (!CRYPTO_THREAD_unlock(lock))
diff --git a/include/internal/hashtable.h b/include/internal/hashtable.h
index 873612d827..c907926dea 100644
--- a/include/internal/hashtable.h
+++ b/include/internal/hashtable.h
@@ -329,4 +329,6 @@ void ossl_ht_value_list_free(HT_VALUE_LIST *list);
  */
 HT_VALUE *ossl_ht_get(HT *htable, HT_KEY *key);

+void ossl_ht_dump(HT *htable, void (*dump_cb)(HT_VALUE *val));
+uint64_t ossl_ht_hash(HT *h, HT_KEY *key);
 #endif
diff --git a/test/lhash_test.c b/test/lhash_test.c
index 76bed5d7e5..51d5b2e42c 100644
--- a/test/lhash_test.c
+++ b/test/lhash_test.c
@@ -470,6 +470,7 @@ end:
 typedef struct test_mt_entry {
     int in_table;
     int pending_delete;
+    uint64_t hash;
 } TEST_MT_ENTRY;

 static HT *m_ht = NULL;
@@ -619,6 +620,13 @@ static void do_mt_hash_work(void)
     }
 }

+static void print_value(HT_VALUE *v)
+{
+    TEST_MT_ENTRY *m = ossl_ht_mt_TEST_MT_ENTRY_from_value(v);
+
+    fprintf(stderr, "ptr:%p, in_table:%d, pending_delete:%d hash: %lu\n", m, m->in_table, m->pending_delete, m->hash);
+}
+
 static int test_hashtable_multithread(void)
 {
     HT_CONFIG hash_conf = {
@@ -630,6 +638,8 @@ static int test_hashtable_multithread(void)
     int ret = 0;
     thread_t workers[4];
     int i;
+    MTKEY key;
+
 #ifdef MEASURE_HASH_PERFORMANCE
     struct timeval start, end, delta;
 #endif
@@ -643,6 +653,13 @@ static int test_hashtable_multithread(void)
     if (!TEST_ptr(m_ht))
         goto end;

+    HT_INIT_KEY(&key);
+    for (i = 0; i < TEST_MT_POOL_SZ; i++) {
+        HT_KEY_RESET(&key);
+        HT_SET_KEY_FIELD(&key, index, i);
+        test_mt_entries[i].hash = ossl_ht_hash(m_ht, TO_HT_KEY(&key));
+    }
+
     worker_lock = CRYPTO_THREAD_lock_new();
     if (worker_lock == NULL)
         goto end_free;
@@ -685,6 +702,7 @@ shutdown:

 end_free:
     shutting_down = 1;
+    ossl_ht_dump(m_ht, print_value);
     ossl_ht_free(m_ht);
 end:
     return ret;

I'm losing the thread of the patches - mostly because I'm applying them manually as I don't have the ability to pull onto that system :(.
__ATOMIC_ACQUIRE is not defined on the platform - where does it normally come from? So we are falling to the bottom case.

This run was before the above patch was applied. I have to apply the patch - couple of hours.
openssl.lhash.log

__ATOMIC_ACQUIRE is defined by libc if your platform supports atomic operations (__atomic_load/store/etc)

hang on, I might see another problem... stand by

when you get a chance:
https://github.com/openssl/openssl/pull/24969
I noticed that ossl_ht_delete was using h->md directly when it arguably shouldn't be.  I can't see how a compiler might do this, but if it recomputes the value of h->md mid function, we might see issues like what you're seeing.  It shouldn't happen, since a delete operation is occuring under the protection of a write lock, but its worth a shot

Applied both crypto/threads_pthread.c and crypto/hashtable/hashtable.c changes to the whole set of patches applied already. About to test.

__ATOMIC_ACQUIRE is defined by libc if your platform supports atomic operations (__atomic_load/store/etc)

Like GNU libc, which we don't have? Or the standard runtime? Either way, we don't have atomics like this, although with the new threading model, perhaps we should.

__ATOMIC_ACQUIRE is defined by libc if your platform supports atomic operations (__atomic_load/store/etc)

Like GNU libc, which we don't have? Or the standard runtime? Either way, we don't have atomics like this, although with the new threading model, perhaps we should.

Yes, this is a gcc-specific non-portable thing.

It might have gotten better, but we're still not there. I don't think the h->md to md change made much of a difference.
openssl.lhash.log

I'm going to start just using the draft pr here https://github.com/openssl/openssl/pull/24969 as giving you iterative patches is making this rather long.  I've augmented that branch with our debugging patch which also prints out the test table index to better identify the failing elements.  If you could pull and run that again.
FWIW, making the ossl_ht_delete change seems to have helped significantly.  while we're still seeing errors, I'm not seeing any more 'broken' hash table entries, which is progress.

I'm going to start just using the draft pr here #24969 as giving you iterative patches is making this rather long. I've augmented that branch with our debugging patch which also prints out the test table index to better identify the failing elements. If you could pull and run that again.

Ack. I missed this update. Will try.

Mostly worked. Two hangs (completion 137 when I killed the process) and a free issue. Looks like we are getting there. Once it goes into the first diagnostic code, everything looks fine. Wondering about the test initialization code.
openssl.lhash.log

ok, pushed a new commit adding dumps of the addresses we are allocating and freeing during runs, shouldn't impact timing much I don't think.  also fixed one missed point where we don't free the unaligned pointer correctly on growth error.
Give that a try and lets see where we land.
Its strange that in the two runs that failed, we had one that failed after the table dump and one before.  Not sure what to make of that in terms of what we're looking for here

ok, pushed a new commit adding dumps of the addresses we are allocating and freeing during runs, shouldn't impact timing much I don't think. also fixed one missed point where we don't free the unaligned pointer correctly on growth error.
Give that a try and lets see where we land.
Its strange that in the two runs that failed, we had one that failed after the table dump and one before. Not sure what to make of that in terms of what we're looking for here

Is it possible that the thread initialization code is somehow randomly not being invoked or has a bug?

@nhorman Well... good question. threadstest works fine - it did not earlier but was fixed by the thread team. I was thinking about a problem in the thread local data. I think this latest run may be more informative. 2 hangs. 1 crash. 7 successful - same as last time, with info.
openssl.lhash.log

good!  I managed to locate another misused free statement on hash table growth.  I'm still concerned about the iterative growth that shouldn't be happening here, but lets try this next one and see where it goes.  I'm predicting this will now fix the freeing of unreserved memory, but we will still have to tackle the hangs (which I suspect is infinite recursive growth, which I can't yet explain)

This looks like progress tracking down the issue. There are some potential loops now.
openssl.lhash.log

yeah, this is progress.  When you hang, it looks like everything is getting hashed into neighborhood 0 or 127, which is decidedly wierd, and causing recursive growth.  We definately shouldn't be hashing into the same neighborhood that much.
Let me stare at the code for a bit

yeah, this is progress. When you hang, it looks like everything is getting hashed into neighborhood 0 or 127, which is decidedly wierd, and causing recursive growth. We definately shouldn't be hashing into the same neighborhood that much. Let me stare at the code for a bit

Bad sign extension on assumed unsigned when it is signed?

Hmmm, maybe?  fnv1a_hash should only be doing arithmetic on unit64_ts, but
it's worth checking.  New update to the branch soon
On Fri, Jul 26, 2024, 5:00 PM Randall S. Becker @.***>
wrote:

yeah, this is progress. When you hang, it looks like everything is getting
hashed into neighborhood 0 or 127, which is decidedly wierd, and causing
recursive growth. We definately shouldn't be hashing into the same
neighborhood that much. Let me stare at the code for a bit
Bad sign extension on assumed unsigned when it is signed?
—
Reply to this email directly, view it on GitHub
https://github.com/openssl/openssl/issues/24913#issuecomment-2253499151,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAH4FXUJKIWSJEYK3UYCOMLZOK2GLAVCNFSM6AAAAABK67PDNOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJTGQ4TSMJVGE
.
You are receiving this because you were mentioned.Message ID:
@.***>

so about the only thing I see is that in fnv1a_hash:
static ossl_unused uint64_t fnv1a_hash(uint8_t *key, size_t len)
{
    uint64_t hash = 0xcbf29ce484222325ULL;
    size_t i;

    for (i = 0; i < len; i++) {
        hash ^= key[i];
        hash *= 0x00000100000001B3ULL;
    }
    return hash;
}

we do an exclusive or of a unit8_t with a uint64_t.  There should be no sign extension there as they're all unsigned types, but maybe.  I've updated the test branch to explicilty cast key to a uint64_t in the exclusive or operation.
I've also trimmed down the debug printfs to focus on the rehashing.
Once again, unto the breach....

Results are interesting this time. I had to stop more processes that were looping.
openssl.lhash.log

well, that is interesting!  It looks like the output of fnv1a_hash occasionally has the lower 32 bits of an entry set to all 0's or all f's. Its almost like its a big endian system and its doing sign extension, but that doesn't make any sense.
Ok, we can use this though.  I'll augment the hash function to flag entries that produce weird outputs like that so we can figure out what inputs are causing this.

well, that is interesting! It looks like the output of fnv1a_hash occasionally has the lower 32 bits of an entry set to all 0's or all f's. Its almost like its a big endian system and its doing sign extension, but that doesn't make any sense.
Ok, we can use this though. I'll augment the hash function to flag entries that produce weird outputs like that so we can figure out what inputs are causing this.

It does look like that. If the variable is signed or is in the middle of an expression, which defaults to int by default, which is signed, then sign extension will happen.

I've pushed a new commit that flags hash values whos lower 32 bits are all 0 or f.  It will dump out the key that produced that value as well, which we can then use to manually try to trace back how we arrived at that value.
One thing I'm thinking about is overflow behavior.  fnv1a hash will definately at some point overflow a uint64_t, which its supposed to do.  As far as I know however, overflow on unsinged values is supposed to be well defined behavior in C (it does modulo math wrapping around back to zero).  I'm starting to wonder if the behavior is different here though.  Can I ask what compiler you are using?

I've pushed a new commit that flags hash values whos lower 32 bits are all 0 or f. It will dump out the key that produced that value as well, which we can then use to manually try to trace back how we arrived at that value.
One thing I'm thinking about is overflow behavior. fnv1a hash will definately at some point overflow a uint64_t, which its supposed to do. As far as I know however, overflow on unsinged values is supposed to be well defined behavior in C (it does modulo math wrapping around back to zero). I'm starting to wonder if the behavior is different here though. Can I ask what compiler you are using?

I'm using c99. Do you have a test expression I can verify?

Not yet ,but with the change I just pushed, if you run it again, should help us catch an input value that we can use to write a test case that will let us trace through the operations of fnv1a_hash to verify each step in the process

Different set of failures this time. We were consistent on which tests failed previously.
openssl.lhash.log

Might want to use %llx for formatting. Should not make a difference. sprintf might sign extend internally.

Just a reminder... the system is big-endian.

ok, good to know.
So these results suggest several things to me:

We never produced the output "WIERD HASH VALUE".  That tells me we never computed a value  with all f's or all 0's in the lower 32 bits.  That in turn implies something is getting corrupted in the hash table proper after the hash has been computed

It only appears (from what we know to occur during hash table growth.

Looking at the expansion code, the only thing that stands out to me is that in the growth operation we do direct assignments of hash values and poitners to the expanded table, instead of doing atomic_stores like we do in the insert path.  I don't see how that would impact anything, as their all just pointers and uint64_ts, but lets make it consistent.

ok, I converted grow_hashtable to use atomic assignments instead of regular assignments.  Not sure how that fixes anything, but it seems to be where the observations are pushing us

hey, if you get a chance, can you also build openssl with -debug and send me the libcrypto.so.3 file?  I'd like to take a look at the disassembly

hey, if you get a chance, can you also build openssl with -debug and send me the libcrypto.so.3 file? I'd like to take a look at the disassembly

41Mb??? Not sure I can.

alternatively you could just build it as such, and run:
objdump -S -d libcrypto.so.3 > ./results

and post the results file, that should be significantly smaller

I think this was an improvement. Might be another sign issue left over. Have a look.
openssl.lhash.log

If I had objdump... much would be different in the universe ;)

ha!  Ok, i have a suspicion, going to try something, standby

Also, I'm building with -d. The builds are with -O0 as a result. Once we get this working, I'll move to the optimizer - I thought the optimizer might be an issue, but ruled that out..

I think maybe ...
  static int __attribute__ ((noinline)) ossl_ht_insert_locked(HT *h, uint64_t hash,
                            ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 540: error(172):
          expected a type specifier

  static int __attribute__ ((noinline)) ossl_ht_insert_locked(HT *h, uint64_t hash,
             ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 540: error(230):
          unnamed prototyped parameters not allowed when body is present

  static int __attribute__ ((noinline)) ossl_ht_insert_locked(HT *h, uint64_t hash,
                                        ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 540: error(222):
          expected a "{"

      struct ht_mutable_data_st *md = ossl_rcu_deref(&h->md);
                                      ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 544: error(114):
          identifier "h" is undefined

      uint64_t neigh_idx = hash & md->neighborhood_mask;
                           ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 545: error(114):
          identifier "hash" is undefined

              if (olddata == NULL) {
                  ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 560: error(114):
          identifier "olddata" is undefined

              newval->last_op = "replace";
              ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 565: error(114):
          identifier "newval" is undefined

              *olddata = (HT_VALUE *)md->neighborhoods[neigh_idx].entries[j].value;
               ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 568: error(114):
          identifier "olddata" is undefined

      newval->last_op = "insert";
      ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 579: error(114):
          identifier "newval" is undefined

      rc = ossl_ht_insert_locked(h, hash, newval, olddata);
           ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 643: error(114):
          identifier "ossl_ht_insert_locked" is undefined

  static void ossl_unused ossl_ht_strcase(char *tgt, const char *src, int len)
                          ^
"/home/ituglib/randall/openssl-klt/include/internal/hashtable.h", line 231: warning(262):
          function "ossl_ht_strcase" was declared but never referenced

  static int __attribute__ ((noinline)) ossl_ht_insert_locked(HT *h, uint64_t hash,
             ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 540: warning(262):
          function "__attribute__" was declared but never referenced

10 errors detected in the compilation of "crypto/hashtable/hashtable.c".

pushed a new change.  I have a hypothesis (and building with -O0 may actually result in this working for you, if it does, I'm taking that as a sign)
Heres my thinking:
The hash table works by creating an array of "neighborhoods", a neighborhood contains a set of pointers to hash table entries (the number being defined by the cpu cache line size)
Normally when you insert an entry, you hash the associated key, and the neighborhood is selected by taking the lowest N bytes of the hash value (i.e. if the hash table is 128 neighborhoods long, the mask for the table is 0x3f, so the neighborhood index is from 0-127.  Easy enough.
When we grow the hashtable, we create a new neighborhood list, and re-insert each entry from the old table, using the updated mask based on the new table size.  also easy
When we finish that, we update the pointer to the neighborhood list, and queue the old table to be cleaned up (this is how rcu does memory cleanup).  Also pretty easy.
When gcc optimizes code, it sometimes inlines functions (pretty typical), and that would be the case here with ossl_ht_insert_locked, as its only called from one other function (ossl_ht_insert)
My hypothesis is that, your compiler is perhaps identifying that when inlining this call, it doesn't need to recompute some of the stack variables that get inlined, most notably the call to:
ossl_rcu_deref(&h->md);

present at the top of ossl_ht_insert_locked.
If your compiler optimized that call out, it would use the old neighborhood table, but the new neighborhood table mask, indexing off the end of the old table, and returning whatever garbage it found there, leading to all sorts of wierd values that have no relevance to the hash table.
If we build with -O0, or force insert_locked to be not inlined, I bet we avoid that, and the probelm goes away

crap, sorry, I'm not sure why you're getting that error, it builds fine for me.  I pushed a new commit that remove the space I shouldn't have in there, that I think will fix it

crap, sorry, I'm not sure why you're getting that error, it builds fine for me. I pushed a new commit that remove the space I shouldn't have in there, that I think will fix it

c99 not gcc. Sorry.

does c99 not do attributes?  I'll have to find another way to ensure noinline if thats the case

attribute is not allowed. It is gcc-specific.
  static int __attribute__((noinline)) ossl_ht_insert_locked(HT *h, uint64_t hash,
                           ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 540: error(172):
          expected a type specifier

  static int __attribute__((noinline)) ossl_ht_insert_locked(HT *h, uint64_t hash,
             ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 540: error(230):
          unnamed prototyped parameters not allowed when body is present

  static int __attribute__((noinline)) ossl_ht_insert_locked(HT *h, uint64_t hash,
                                       ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 540: error(222):
          expected a "{"

      struct ht_mutable_data_st *md = ossl_rcu_deref(&h->md);
                                      ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 544: error(114):
          identifier "h" is undefined

      uint64_t neigh_idx = hash & md->neighborhood_mask;
                           ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 545: error(114):
          identifier "hash" is undefined

              if (olddata == NULL) {
                  ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 560: error(114):
          identifier "olddata" is undefined

              newval->last_op = "replace";
              ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 565: error(114):
          identifier "newval" is undefined

              *olddata = (HT_VALUE *)md->neighborhoods[neigh_idx].entries[j].value;
               ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 568: error(114):
          identifier "olddata" is undefined

      newval->last_op = "insert";
      ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 579: error(114):
          identifier "newval" is undefined

      rc = ossl_ht_insert_locked(h, hash, newval, olddata);
           ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 643: error(114):
          identifier "ossl_ht_insert_locked" is undefined

  static void ossl_unused ossl_ht_strcase(char *tgt, const char *src, int len)
                          ^
"/home/ituglib/randall/openssl-klt/include/internal/hashtable.h", line 231: warning(262):
          function "ossl_ht_strcase" was declared but never referenced

  static int __attribute__((noinline)) ossl_ht_insert_locked(HT *h, uint64_t hash,
             ^
"/home/ituglib/randall/openssl-klt/crypto/hashtable/hashtable.c", line 540: warning(262):
          function "__attribute__" was declared but never referenced

I can force the build without inline, but I doubt this is the issue. -Wnoline is how suppression is done. By default, inlining is not explicitly assumed unless the optimizer decides to do it but never in -O0.

Using export$ can force a function to never be inlined.

I would be curious to see the result inside and out of the function causing issues to see whether this is an inline issue.

I can augment the function to show that I think.  I'm away from my desk at
the moment, but.will.put something together in the am
On Sat, Jul 27, 2024, 5:50 PM Randall S. Becker @.***>
wrote:

I would be curious to see the result inside and out of the function
causing issues to see whether this is an inline issue.
—
Reply to this email directly, view it on GitHub
https://github.com/openssl/openssl/issues/24913#issuecomment-2254262862,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAH4FXTJ5PIQMFBFN2VNHF3ZOQI3JAVCNFSM6AAAAABK67PDNOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJUGI3DEOBWGI
.
You are receiving this because you were mentioned.Message ID:
@.***>

Works. for me. I have some family things tomorrow, so I'm not sure when. I am concerned about losing access to the test system imminently, though. Doing my best to keep access as long as I can.

had a moment, so I updated our debug branch to test the hypothesis. It doesn't do anything about inlining or not, but it should catch if we inappropriately use a neighborhood list after its retired.  Enjoy your family time, talk soon

Results look interesting. One hard loop, and a few hangs at the same spot after a free.
openssl.lhash.log

this is making less and less sense

this is making less and less sense

Trying something... I don't think it is making less sense, but...

This might help. I do not know how extensive:
            if (newi == 0ULL || newi == 127ULL) {

Because this is a long long

This might help. I do not know how extensive:
            if (newi == 0ULL || newi == 127ULL) {

Because this is a unsigned long long compare, the constants are not being represented identically to the variable when pushed into registers for the compare - I have seen this before from c89, c99, and c11 on multiple platforms including NonStop and S/390. Being explicit with the contents makes a big difference. I did not catch everything, but many of the unexplained Odd hash value! went away.

I do not get the following:
        /* round up to the next power of 2 */
        new->wpd.neighborhood_len--;
        new->wpd.neighborhood_len |= new->wpd.neighborhood_len >> 1;
        new->wpd.neighborhood_len |= new->wpd.neighborhood_len >> 2;
        new->wpd.neighborhood_len |= new->wpd.neighborhood_len >> 4;
        new->wpd.neighborhood_len |= new->wpd.neighborhood_len >> 8;
        new->wpd.neighborhood_len |= new->wpd.neighborhood_len >> 16;
        new->wpd.neighborhood_len++;

Rounding up to the next power of 2... I don't quite follow.

Where it is failing currently is:
02-test_lhash.t ..
# The results of this test will end up in test-runs/test_lhash
1..1
    # Subtest: ../../test/lhash_test
    1..5
    ok 1 - test_int_lhash
    ok 2 - test_stress
    ok 3 - test_int_hashtable
    ok 4 - test_hashtable_stress