Closed edquist closed 2 years ago
Do you potentially see an issue with the line here: https://github.com/scitokens/scitokens-cpp/blob/bddd5f0edf3ad070bfe8255f8c73a8eb7fd77460/src/scitokens_cache.cpp#L117
Do you potentially see an issue with the line here
No, i do not, though admittedly I'm new to this codebase.
In particular,
db
is a local (ie not global) pointer variable in the scitokens::Validator::store_public_keys
function; it doesn't look like it gets passed anywhere else that stores it, so nothing else should be able to access this db handle anyway after the return, if we didn't close it.
the line you linked is part of remove_issuer_entry
, which scitokens::Validator::store_public_keys
calls earlier, before the db close I'm adding, so it's not like this will be run on the same db handle after closing it
further, the line you linked is wrapped in a if (new_transaction)
, but scitokens::Validator::store_public_keys
calls this function (remove_issuer_entry
) with false
passed in for new_transaction
, so in this case the line you are referring to doesn't get run anyway.
... let me know if you see something though!
(attn: @brianhlin @djw8605, i don't have permissions to assign a reviewer for this)
I'm seeing condor_schedd accumulating open fds pointing to
/var/lib/condor/.cache/scitokens/scitokens_cpp.sqllite
(thousands), without ever getting closed.I looked around the sources here and every other
return
in a function that opens this db is preceeded by asqlite3_close(db);
. Presumably this one here is the source of the leak and just got missed.