From 00a0f76f38fdd68622f46c7ffe5f88668393c01b Mon Sep 17 00:00:00 2001 From: Salil Chandra Date: Tue, 7 Jul 2026 10:55:11 -0400 Subject: [PATCH] {184305174}: Close displaced DB_AM_RECOVER handles in __ufid_add_dbp When __ufid_add_dbp is called for a file that already has an entry in the ufid-hash, the existing handle is displaced by the new one. The previous code cleared the added_to_ufid flag on the old handle but did not close it, causing a file descriptor leak. Root Cause: - During replication, dbreg_register log records cause __dbreg_do_open to be called, creating a new DB_AM_RECOVER handle for the file - __ufid_add_dbp replaces the previous handle in the ufid-hash but only clears its added_to_ufid flag without closing it - The displaced handle remains in gbl_db_open_list holding an open file descriptor - Each subsequent dbreg_register for the same file displaces and leaks another handle - When delfiles deletes old file versions, these leaked handles keep the inodes alive, causing files to appear as "(deleted)" in lsof Fix: - When displacing an existing DB_AM_RECOVER handle, update the ufid-hash to point to the new handle first, then close the old handle - The close removes the old handle from gbl_db_open_list, releasing the file descriptor Signed-off-by: Salil Chandra --- berkdb/dbreg/dbreg_rec.c | 8 +++ berkdb/dbreg/dbreg_util.c | 15 ++++- db/db_tunables.c | 1 + db/db_tunables.h | 2 + tests/berkdb_file_leaks_rep.test/runit | 2 +- tests/ufid_handle_leak.test/Makefile | 1 + tests/ufid_handle_leak.test/lrl.options | 5 ++ tests/ufid_handle_leak.test/runit | 73 +++++++++++++++++++++++++ 8 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 tests/ufid_handle_leak.test/Makefile create mode 100644 tests/ufid_handle_leak.test/lrl.options create mode 100755 tests/ufid_handle_leak.test/runit diff --git a/berkdb/dbreg/dbreg_rec.c b/berkdb/dbreg/dbreg_rec.c index f415748c9a..d076feea22 100644 --- a/berkdb/dbreg/dbreg_rec.c +++ b/berkdb/dbreg/dbreg_rec.c @@ -61,6 +61,8 @@ static const char revid[] = "$Id: dbreg_rec.c,v 11.120 2003/10/27 15:54:31 sue E static int __dbreg_open_file __P((DB_ENV *, DB_TXN *, __dbreg_register_args *, void *)); +int gbl_test_force_dbreg_reopen = 0; + int __dbreg_register_print(DB_ENV *dbenv, DBT *dbtp, DB_LSN *lsnp, db_recops notused2, void *notused3); @@ -427,6 +429,12 @@ __dbreg_open_file(dbenv, txn, argp, info) goto reopen; } + if (gbl_test_force_dbreg_reopen) { + MUTEX_THREAD_UNLOCK(dbenv, lp->mutexp); + (void)__dbreg_revoke_id(dbp, 0, DB_LOGFILEID_INVALID); + goto reopen; + } + /* * We should only get here if we already have the * dbp from an openfiles pass, in which case, what's diff --git a/berkdb/dbreg/dbreg_util.c b/berkdb/dbreg/dbreg_util.c index fd68af30dc..9cc11c59bd 100644 --- a/berkdb/dbreg/dbreg_util.c +++ b/berkdb/dbreg/dbreg_util.c @@ -456,7 +456,20 @@ __ufid_add_dbp(dbenv, dbp) master and later assigns a dbreg ID to a btree (see __dbreg_lazy_id). In this case, we need to take the old dbp off the hashtable so that __db_close() doesn't mistakenly clear the new dbp. */ - ufid->dbp->added_to_ufid = 0; + DB *old_dbp = ufid->dbp; + old_dbp->added_to_ufid = 0; + if (F_ISSET(old_dbp, DB_AM_RECOVER)) { + /* Close the displaced DB_AM_RECOVER handle to prevent file + * descriptor leaks. These handles accumulate when a regular + * handle displaces a recovery handle in the ufid-hash without + * closing it. When delfiles later deletes these old file + * versions, the leaked handles keep the inodes alive. */ + ufid->dbp = dbp; + dbp->added_to_ufid = 1; + Pthread_mutex_unlock(&dbenv->ufid_to_db_lk); + __db_close(old_dbp, NULL, DB_NOSYNC); + return ret; + } } } else { if ((ret = __os_malloc(dbenv, sizeof(*ufid), &ufid)) != 0) { diff --git a/db/db_tunables.c b/db/db_tunables.c index ec6c196deb..928668cc99 100644 --- a/db/db_tunables.c +++ b/db/db_tunables.c @@ -648,6 +648,7 @@ extern int gbl_accept_headroom; extern int gbl_db_track_open; extern int gbl_clear_ufid_on_db_close; extern int gbl_get_peer_fqdn; +extern int gbl_test_force_dbreg_reopen; int parse_int64(const char *value, int64_t *num); diff --git a/db/db_tunables.h b/db/db_tunables.h index 3f8d9a5ec3..650e108c2d 100644 --- a/db/db_tunables.h +++ b/db/db_tunables.h @@ -2689,4 +2689,6 @@ REGISTER_TUNABLE("clear_ufid_on_db_close", "Clear ufid hash on db->close", TUNAB INTERNAL, NULL, NULL, NULL, NULL); REGISTER_TUNABLE("get_peer_fqdn", "When set, use peer's FQDN", TUNABLE_BOOLEAN, &gbl_get_peer_fqdn, INTERNAL, NULL, NULL, NULL, NULL); +REGISTER_TUNABLE("test_force_dbreg_reopen", "Force dbreg reopen to test ufid displacement", TUNABLE_INTEGER, + &gbl_test_force_dbreg_reopen, INTERNAL, NULL, NULL, NULL, NULL); #endif /* _DB_TUNABLES_H */ diff --git a/tests/berkdb_file_leaks_rep.test/runit b/tests/berkdb_file_leaks_rep.test/runit index 312cfe03d7..b01fffdbd0 100755 --- a/tests/berkdb_file_leaks_rep.test/runit +++ b/tests/berkdb_file_leaks_rep.test/runit @@ -24,7 +24,7 @@ dbnm=$1 master=`cdb2sql --tabs ${CDB2_OPTIONS} $dbnm default "select host from comdb2_cluster where is_master='Y'"` replicant=`cdb2sql --tabs ${CDB2_OPTIONS} $dbnm default "select comdb2_host()"` -the_other_replicant=`cdb2sql --tabs ${CDB2_OPTIONS} $dbnm default "SELECT host FROM comdb2_cluster WHERE host <> '$master' AND host <> '$replicant'"` +the_other_replicant=`cdb2sql --tabs ${CDB2_OPTIONS} $dbnm default "SELECT host FROM comdb2_cluster WHERE host <> '$master' AND host <> '$replicant' LIMIT 1"` cdb2sql ${CDB2_OPTIONS} $dbnm default "CREATE TABLE t1 (i integer, b blob)" sleep 2 # wait for the checkpoint thread to do its 1st checkpoint diff --git a/tests/ufid_handle_leak.test/Makefile b/tests/ufid_handle_leak.test/Makefile new file mode 100644 index 0000000000..59da0287f7 --- /dev/null +++ b/tests/ufid_handle_leak.test/Makefile @@ -0,0 +1 @@ +include $(TESTSROOTDIR)/testcase.mk diff --git a/tests/ufid_handle_leak.test/lrl.options b/tests/ufid_handle_leak.test/lrl.options new file mode 100644 index 0000000000..2696ad2fa3 --- /dev/null +++ b/tests/ufid_handle_leak.test/lrl.options @@ -0,0 +1,5 @@ +setattr CHECKPOINTTIME 9999999 +setattr MEMPTRICKLEPERCENT 10 +track_db_open 1 +test_force_dbreg_reopen 1 +dtastripe 8 diff --git a/tests/ufid_handle_leak.test/runit b/tests/ufid_handle_leak.test/runit new file mode 100755 index 0000000000..1aa6f62c94 --- /dev/null +++ b/tests/ufid_handle_leak.test/runit @@ -0,0 +1,73 @@ +#!/usr/bin/env bash +bash -n "$0" | exit 1 + +# Test that DB_AM_RECOVER handles displaced from the ufid-hash by +# __ufid_add_dbp are properly closed, preventing file descriptor leaks. +# +# Uses the test_force_dbreg_reopen tunable to force __dbreg_open_file to +# skip its early-return optimization when a dbreg slot already has a +# matching handle. This causes __dbreg_do_open to create a DB_AM_RECOVER +# handle that displaces the existing handle in the ufid-hash — the exact +# scenario that occurs in prod during schema change replay. +# +# Without the fix in __ufid_add_dbp, each displacement leaks a handle +# (visible as growth in dumpopendbs). With the fix, displaced DB_AM_RECOVER +# handles are closed and the count stays stable. + +source ${TESTSROOTDIR}/tools/cluster_utils.sh + +[ -z "${CLUSTER}" ] && { echo "Test requires a cluster"; exit 0; } + +dbnm=$1 + +master=$(cdb2sql --tabs ${CDB2_OPTIONS} $dbnm default \ + "SELECT host FROM comdb2_cluster WHERE is_master='Y'") +replicant=$(cdb2sql --tabs ${CDB2_OPTIONS} $dbnm default \ + "SELECT host FROM comdb2_cluster WHERE is_master='N' LIMIT 1") + +echo "master=$master replicant=$replicant" + +cdb2sql ${CDB2_OPTIONS} $dbnm default "CREATE TABLE t1 (i INT, b BLOB)" +sleep 1 +cdb2sql ${CDB2_OPTIONS} $dbnm default "INSERT INTO t1 VALUES(1, x'DEADBEEF')" +sleep 2 + +# Measure BEFORE any explicit checkpoint. The test_force_dbreg_reopen +# tunable hasn't fired yet because CHECKPOINTTIME is set very high. +before=$(cdb2sql --tabs ${CDB2_OPTIONS} $dbnm --host $replicant \ + "EXEC PROCEDURE sys.cmd.send('bdb dumpopendbs')" | grep -c 'XXX\.t1') +echo "Open t1 handles on $replicant before checkpoint: $before" + +# Checkpoint generates dbreg_chkpnt log records. With test_force_dbreg_reopen, +# the replicant's __dbreg_open_file skips its early return and calls +# __dbreg_do_open, creating a DB_AM_RECOVER handle that displaces the +# existing regular handle in the ufid-hash. +cdb2sql $dbnm --host $master "EXEC PROCEDURE sys.cmd.send('bdb checkpoint')" +sleep 2 + +# TRUNCATE creates new btree files, generating more dbreg_register records. +for i in $(seq 1 3); do + cdb2sql ${CDB2_OPTIONS} $dbnm default "TRUNCATE TABLE t1" + cdb2sql $dbnm --host $master "EXEC PROCEDURE sys.cmd.send('bdb checkpoint')" + sleep 2 +done + +after=$(cdb2sql --tabs ${CDB2_OPTIONS} $dbnm --host $replicant \ + "EXEC PROCEDURE sys.cmd.send('bdb dumpopendbs')" | grep -c 'XXX\.t1') +echo "Open t1 handles on $replicant after checkpoint+truncates: $after" + +# With the fix: displaced DB_AM_RECOVER handles are closed → count stable +# Without the fix: each checkpoint + truncate leaks handles → count grows +threshold=$(( before + (before / 4) + 2 )) +if [ "$after" -gt "$threshold" ]; then + echo "FAIL: handle count grew from $before to $after (threshold $threshold)" >&2 + echo "Leaked DB_AM_RECOVER handles detected" >&2 + echo "--- dumpopendbs t1 entries ---" + cdb2sql --tabs ${CDB2_OPTIONS} $dbnm --host $replicant \ + "EXEC PROCEDURE sys.cmd.send('bdb dumpopendbs')" | grep 'XXX\.t1' + echo "--- end ---" + exit 1 +fi + +echo "PASS: handle count stable ($before → $after, threshold $threshold)" +exit 0