WIP: Update external ACL cache to ClpMap #1734
Conversation
yadij
changed the title
| #endif | ||
|
|
||
| // XXX: This state->def access conflicts with the cbdata validity check | ||
| // below. | ||
| dlinkDelete(&state->list, &state->def->queue); | ||
|
|
||
| ExternalACLEntryPointer entry; | ||
| if (cbdataReferenceValid(state->def)) | ||
| entry = external_acl_cache_add(state->def, state->key, entryData); |
There was a problem hiding this comment.
AFAICT, official code unconditinoally creates ExternalACLEntry (that stores helper lookup info) and passes it to ACLExternal::aclMatchExternal() via ACLFilledChecklist::extacl_entry hack. This approach is ugly as hell, but it does not rely on the cache to keep/pass fresh lookup information: Despite its name, external_acl_cache_add() may actually add nothing to the cache.
PR code removes ACLFilledChecklist::extacl_entry hack and relies on the cache to keep/pass fresh lookup information. This approach is conceptually wrong: Caching is an optimization that we cannot rely on to pass information. The code has to be written so that it continues to work correctly when caching is disabled or otherwise impossible.
Before refactoring to fix this problem, please create a test case that will fail (with the current PR code) when external ACL caching is disabled.
rousskov
added
the
S-waiting-for-author
No description provided.