URL:
https://github.com/SSSD/sssd/pull/5375
Title: #5375: kcm: improve performance for large ccaches
alexey-tikhonov commented:
"""
> Could you please explain claim "2)" in the
"sss_ptr_hash: fix double free for circular dependencies" patch?
Honestly, I can not. I already forgot the details and it is not directly reproducable.
The reproducer was to run `kvno` to obtain a service ticket. It called
`GET_CRED_UUID_LIST` several times on the same connection so with hit
`kcm_creds_to_table()` multiple times on non-empty table (which is perfectly valid case as
the comment describes). This lead to the first issue, when hash_delete(old) was called
from hasn_enter() to overwrite existing entry. I hit the second issue (crash in talloc)
with few non-final patches for the first issue so I can't tell you how to reproduce.
There was some catch in it bound to the fact that an entry is being overwritten. I can
remove it from the description if you want?
I agree that `overwriting existing entry` results in double `hash_delete()` if custom
delete_cb calls talloc_free(payload). I'm not sure if `delete_in_progress` is the best
way to deal with it (can't propose anything better atm), but I agree this check in
`sss_ptr_hash_delete_cb` resolves this issue.
With regards to additional check in `sss_ptr_hash_value_destructor`: I think it's not
required (and test added by you runs fine without this check). But taking into account
comment text along this check - I'm ok with it.
Nonetheless, I still think part (2) of commit message is confusing so if not possible to
update, I would propose to delete it.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/5375#issuecomment-736784240