User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:220.127.116.11) Gecko/20070713 Firefox/18.104.22.168 Build Identifier: ldap_memcache_flush() seems to only clear out cache items that contain messages of search entry type with a DN that matches the base that was based to ldap_memcache_flush(). But searches that didn't find an entry would only cache a search result message and no search entries. In memcache.c:memcache_access() when "mode == MEMCACHE_ACCESS_FLUSH" it's skipping the cache slot if (!NSLDAPI_IS_SEARCH_ENTRY( pMsg->lm_msgtype )). I think we should flush it if there are no search entries, and the base in the search results header matches the base passed in to ldap_memcache_flush(). Reproducible: Always
The memcache code is not widely used and not well tested (it was written by a messaging server engineer a long time ago, but most people are better off implementing their own cache inside their application). Do you know of any mainstream applications that use it? In any case, this does sound like a bug. Since there is not much documentation in the code, it is hard to tell what the original author's intent was though.
Not mainstream but there's a proprietary application (not from me) used by many institutions around the world. I think I will need to fix it, maybe I could add an additional memcache_access() mode like MEMCACHE_ACCESS_FLUSH_EXT so I don't break the existing MEMCACHE_ACCESSS_FLUSH behavior in case someone relies on it?
(In reply to comment #2) > Not mainstream but there's a proprietary application (not from me) used by many > institutions around the world. Is the application open source? Where would one get a copy? > ... I think I will need to fix it, maybe I could > add an additional memcache_access() mode like MEMCACHE_ACCESS_FLUSH_EXT so I > don't break the existing MEMCACHE_ACCESSS_FLUSH behavior in case someone > relies on it? Good idea. Maybe use the name MEMCACHE_ACCESS_FLUSH_RESULTS (or something else that is more descriptive than ..._EXT).
Created attachment 275868 [details] [diff] [review] patch proposal The application is closed source unfortunately, I haven't seen it either. This patch adds a new LDAP API call, ldap_memcache_flush_results() which works the same as ldap_memcache_flush() except it'll also clear out results from search requests that didn't return any entries.
The patch looks good to me. Do you have any tests for the memcache code that could be added to the source tree? This really seems like a bug or odd feature that we should have known about long ago....
Assignee: mcs → ulf
Status: UNCONFIRMED → NEW
Ever confirmed: true
Can you add a + for the review if it's OK? Also, Rich and Nathan, any concerns about the changes? Regarding tests, I was using a modified copy of the csearch example but I don't think the examples programs are a good place for tests. What did you have in mind?
Comment on attachment 275868 [details] [diff] [review] patch proposal Forgot to mark patch as OK (oops).
Attachment #275868 - Flags: review?(mcs) → review+
(In reply to comment #6) > Regarding tests, I was using a modified copy of the csearch example but I don't > think the examples programs are a good place for tests. What did you have in > mind? It would be nice to start creating tests that we can commit to CVS and run. But we have never gotten a lot of traction in that area for the LDAP C SDK. If the changes to csearch are not too ugly, attach them to this bug so we at least have a way to revisit this bug if we need to do so.
Checking in directory/c-sdk/ldap/libraries/libldap/memcache.c; /cvsroot/mozilla/directory/c-sdk/ldap/libraries/libldap/memcache.c,v <-- memcache.c new revision: 5.7; previous revision: 5.6 done Checking in directory/c-sdk/ldap/include/ldap-extension.h; /cvsroot/mozilla/directory/c-sdk/ldap/include/ldap-extension.h,v <-- ldap-extension.h new revision: 5.9; previous revision: 5.8 done
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.