https://bugs.openldap.org/show_bug.cgi?id=10608

Issue ID: 10608 Summary: back-mdb: cached per-thread read txns of a database deleted via cn=config are never released -> err=80 "internal error" on new databases Product: OpenLDAP Version: 2.6.13 Hardware: x86_64 OS: Linux Status: UNCONFIRMED Keywords: needs_review Severity: normal Priority: --- Component: backends Assignee: bugs@openldap.org Reporter: coding@markus-mazurczak.de Target Milestone: ---

Created attachment 1218 --> https://bugs.openldap.org/attachment.cgi?id=1218&action=edit Patch: 0002-back-mdb-purge-reader-keys-on-db-close

SUMMARY =======

When a back-mdb database is removed at runtime with an ldapdelete of its olcDatabase entry in cn=config, only the thread that performs the delete releases its cached read transaction for that database. Every other worker thread keeps its cached entry for good: the key is the (now freed) MDB_env pointer and the value a read txn of the closed environment.

The leak accumulates with every database that is created, read and deleted. Once a worker thread has collected about 32 such dead entries, operations on any NEWLY created database fail in that thread with

RESULT tag=97 err=80 qtime=0.000020 etime=0.000082 text=internal error

(on glibc builds the result code is err=12, see below). Databases whose entry was cached before the slots filled up keep working, so the failure looks random: it hits only new databases and only some threads.

This is the typical life cycle of a multi-tenant directory that provisions one mdb database per tenant through cn=config. In our production server the first bind to a freshly provisioned tenant failed sporadically with err=80 after a few hundred tenant create/delete cycles; restarting slapd made it go away until the slots filled again.

ANALYSIS ========

mdb_opinfo_get() caches one read txn per worker thread and database in the thread-pool context, keyed by the environment pointer (servers/slapd/back-mdb/id2entry.c, openldap-2.6.13):

790 if ( ldap_pvt_thread_pool_getkey( ctx, mdb->mi_dbenv, &data, NULL ) ) { ... mdb_txn_begin( ..., MDB_RDONLY, &moi->moi_txn ) ... 808 if ( ( rc = ldap_pvt_thread_pool_setkey( ctx, mdb->mi_dbenv, data, mdb_reader_free, NULL, NULL ) ) ) { ... 812 Debug( LDAP_DEBUG_ANY, "mdb_opinfo_get: thread_pool_setkey failed err (%d)\n", rc ); return rc; } } else { moi->moi_txn = data; 818 renew = 1; } ... 827 rc = mdb_txn_renew( moi->moi_txn ); 828 assert(!rc);

When the database is closed, mdb_db_close() releases only the key of the calling thread (servers/slapd/back-mdb/init.c):

349 mdb_reader_flush( mdb->mi_dbenv );

mdb_reader_flush() (id2entry.c:687) looks up ldap_pvt_thread_pool_context(), i.e. the current thread. The only place in back-mdb that clears the key in all threads is the olcDbDirectory handler (config.c:630), which calls ldap_pvt_thread_pool_purgekey() -- documented as "May only be called while the pool is paused" (libraries/libldap/tpool.c:1424).

Deleting a database through cn=config does not purge. config_back_delete() pauses the server and then calls

bconfig.c:6881 ce->ce_be->bd_info->bi_db_close( ce->ce_be, NULL ); bconfig.c:6884 backend_destroy_one( ce->ce_be, 1);

so mdb_db_close() runs while the pool is paused, but still only flushes the deleting thread. Every other thread that ever read from that database keeps a dangling {env pointer -> txn} entry.

Two consequences, both observed:

1. Slot exhaustion. A thread context has MAXKEYS = 32 key slots (tpool.c:52). Dead entries are never removed, so after roughly 32 deleted databases per thread, ldap_pvt_thread_pool_setkey() returns ENOMEM (tpool.c:1412) for every new database. mdb_opinfo_get() returns that value and mdb_bind() answers with it and "internal error" (bind.c:62). The log shows

mdb_opinfo_get: thread_pool_setkey failed err (12) RESULT tag=97 err=12 ... text=internal error

The raw errno ends up as the LDAP result code (12 is unavailableCriticalExtension), which is a small secondary issue.

2. Stale txn reuse (ABA). With an allocator that hands out a freed address again right away (musl), the MDB_env of a newly created database often gets the address of a deleted one. getkey() then finds the dead entry, mdb_opinfo_get() takes the renew path with a txn that belongs to the closed environment, and the assert(!rc) after mdb_txn_renew() is compiled out in release builds. The next access fails; with a debug print in mdb_bind() we see

bind/dn2entry: rc=MDB_BAD_DBI: The specified DBI handle was closed/changed unexpectedly(-30780) RESULT tag=97 err=80 ... text=internal error

This is what production (Alpine) shows. Continuing with a txn object of a destroyed environment is also a memory-safety problem in itself; we did not see an AddressSanitizer report in our runs (ASan's quarantine prevents the address reuse), so we do not claim more than the observed behaviour.

REPRODUCTION ============

Self-contained script: stale-reader-test.sh (attached). It needs slapd, slapadd, slappasswd and the ldap client tools, and runs in a throw-away container:

1. start slapd with a cn=config database and olcThreads: 2 (or 16) 2. create a long-lived database "dc=steady" and bind to it once 3. repeat 60 times: - ldapadd olcDatabase=mdb (olcSuffix dc=cN, own olcDbDirectory) - add dc=cN and cn=admin,dc=cN (simpleSecurityObject) - 8 simple binds as cn=admin,dc=cN (spread over the worker threads) - one bind to dc=steady - ldapdelete the olcDatabase entry of dc=cN, remove its directory 4. count failed binds

# Alpine 3.23 package build of 2.6.13, back_mdb as module podman run --rm --entrypoint bash -v ./stale-reader-test.sh:/t.sh:ro,z \ -e CYCLES=60 -e BINDS=8 -e MODULES=1 <image> /t.sh

# source build (back-mdb built in), e.g. with AddressSanitizer podman run --rm -v ./stale-reader-test.sh:/t.sh:ro,z \ -e CYCLES=60 -e BINDS=8 -e THREADS=4 -e MODULES=0 <image> bash /t.sh

Results. All builds contain the ITS#10607 patch (without it, concurrent cn=config writes crash slapd); the pairs differ only in the attached patch:

build threads cycles x binds failed binds first ---------------------------- ------- -------------- ------------- ----- Alpine 2.6.13 (musl) 2 60 x 8 137 (err=80) 34 Alpine 2.6.13 (musl) 16 200 x 40 4079 (err=80) 34 Alpine 2.6.13 + patch 16 200 x 40 0 - Alpine 2.6.13 + patch 2 60 x 8 0 - source build, glibc + ASan 4 60 x 8 57 (err=12) 51 source build, glibc + ASan + patch 4 60 x 8 0 -

Binds to the long-lived database dc=steady never failed in any run, which is why the problem is easy to miss: only new databases are affected.

On our production server (Alpine, 16 threads) the same sequence with 80 cycles x 40 binds against the patched build ran 3200 binds without a single failure and without any err=80 in the log.

The first failure appears after about 30 cycles because every cycle leaves one dead key in each thread that served a read from that database, and a thread context also holds a few unrelated keys (e.g. the search stack).

PATCH =====

In mdb_db_close(), purge the key from all threads when the pool is paused, which is the case when the database is deleted via cn=config; otherwise keep the existing per-thread flush (shutdown, slap tools). This mirrors what the olcDbDirectory handler in back-mdb/config.c already does.

if ( ( slapMode & SLAP_SERVER_MODE ) && ldap_pvt_thread_pool_pausing( &connection_pool ) > 0 ) ldap_pvt_thread_pool_purgekey( mdb->mi_dbenv ); else mdb_reader_flush( mdb->mi_dbenv );

ldap_pvt_thread_pool_purgekey() calls the key's free function (mdb_reader_free -> mdb_txn_abort) for every thread while the environment is still open, so the cached read txns are ended before mdb_env_close() instead of outliving it.

The patch applies with -p1 to openldap-2.6.13 (on its own or on top of the ITS#10607 patch), to OPENLDAP_REL_ENG_2_6 and to master (hunk offset 30 lines).

Verification with the patch applied: - stale-reader-test.sh: 0 failed binds (tables above), AddressSanitizer clean - the concurrent cn=config load test from ITS#10607 (4 parallel create/overlay/modify/delete loops plus continuous binds to an unrelated database), which exercises the new purgekey path under concurrency: Alpine build, 150 s: 13083 database cycles, 0 of 19606 binds rejected, no crash glibc + ASan, 120 s: 9579 database cycles, 0 of 14565 binds rejected, AddressSanitizer clean - server restart with existing databases and our orphaned-directory start test: unchanged behaviour

Open points, happy to adjust: - The check could live in config_back_delete() instead (it knows it is removing a database while paused); putting it into mdb_db_close() keeps the knowledge about the key inside back-mdb, like config.c:630. - mdb_bind() and friends return the raw errno from mdb_opinfo_get() as LDAP result code (err=12). Mapping it to LDAP_OTHER would be more accurate; not part of this patch.