You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by "William A. Rowe, Jr." <wr...@rowe-clan.net> on 2001/05/01 03:19:06 UTC
Why lock pagf?
Greg (and dbm folks),
did you have anything specific in mind when you choose pagf rather
than dirf? I'm asking, since my cache-refresh patch to fstat the
mtime to determine cache validity aught to correspond to the fstat
we already need for the dirf. To wrap these all against the same
file handle would be a good thing.
Any reason not to lock against dirf instead?
The upshot - realized if we will cache stuff across processes for
mod_auth_digest, and if users will update passwords in dbm, we don't
see changing 'trusting' the cache.
Bill
Index: dbm/sdbm/sdbm.c
===================================================================
RCS file: /home/cvs/apr-util/dbm/sdbm/sdbm.c,v
retrieving revision 1.14
diff -u -r1.14 sdbm.c
--- dbm/sdbm/sdbm.c 2001/04/30 19:03:47 1.14
+++ dbm/sdbm/sdbm.c 2001/05/01 01:17:14
@@ -115,9 +115,9 @@
{
apr_sdbm_t *db = data;
- (void) apr_file_close(db->dirf);
if (!(db->flags & SDBM_SHARED))
(void) sdbm_unlock(db);
+ (void) apr_file_close(db->dirf);
(void) apr_file_close(db->pagf);
free(db);
@@ -163,6 +163,10 @@
* If we fail anywhere, undo everything, return NULL.
*/
+ if ((status = apr_file_open(&db->dirf, dirname, flags, perms, p))
+ != APR_SUCCESS)
+ goto error;
+
if ((status = apr_file_open(&db->pagf, pagname, flags, perms, p))
!= APR_SUCCESS)
goto error;
@@ -171,7 +175,10 @@
!= APR_SUCCESS)
goto error;
- if ((status = apr_file_open(&db->dirf, dirname, flags, perms, p))
+ /*
+ * need the dirfile size to establish max bit number.
+ */
+ if ((status = apr_file_info_get(&finfo, APR_FINFO_SIZE, db->dirf))
!= APR_SUCCESS)
goto error;
@@ -183,19 +190,10 @@
goto error;
/*
- * need the dirfile size to establish max bit number.
- */
- if ((status = apr_file_info_get(&finfo, APR_FINFO_SIZE, db->dirf))
- != APR_SUCCESS)
- goto error;
-
- /*
* zero size: either a fresh database, or one with a single,
* unsplit data page: dirpage is all zeros.
*/
- db->dirbno = (!finfo.size) ? 0 : -1;
- db->pagbno = -1;
- db->maxbno = finfo.size * BYTESIZ;
+ SDBM_INVALIDATE_CACHE(db, finfo);
/* (apr_pcalloc zeroed the buffers) */
@@ -207,10 +205,11 @@
return APR_SUCCESS;
error:
+ if (db->dirf && db->pagf)
+ (void) sdbm_unlock(db);
if (db->dirf != NULL)
(void) apr_file_close(db->dirf);
if (db->pagf != NULL) {
- (void) sdbm_unlock(db);
(void) apr_file_close(db->pagf);
}
free(db);
Index: dbm/sdbm/sdbm_lock.c
===================================================================
RCS file: /home/cvs/apr-util/dbm/sdbm/sdbm_lock.c,v
retrieving revision 1.6
diff -u -r1.6 sdbm_lock.c
--- dbm/sdbm/sdbm_lock.c 2001/04/30 17:16:19 1.6
+++ dbm/sdbm/sdbm_lock.c 2001/05/01 01:17:14
@@ -52,25 +52,38 @@
* <http://www.apache.org/>.
*/
+#include "apr_file_info.h"
#include "apr_file_io.h"
#include "apr_sdbm.h"
#include "sdbm_private.h"
+#include "sdbm_tune.h"
/* NOTE: this function blocks until it acquires the lock */
apr_status_t sdbm_lock(apr_sdbm_t *db, int exclusive)
{
- int type;
+ apr_status_t status = apr_file_lock(db->dirf,
+ exclusive ? APR_FLOCK_EXCLUSIVE
+ : APR_FLOCK_SHARED);
- if (exclusive)
- type = APR_FLOCK_EXCLUSIVE;
- else
- type = APR_FLOCK_SHARED;
- return apr_file_lock(db->pagf, type);
+ if (status == APR_SUCCESS && (db->flags & SDBM_SHARED))
+ {
+ apr_finfo_t finfo; /* track the page file time info */
+ /*
+ * zero size: either a fresh database, or one with a single,
+ * unsplit data page: dirpage is all zeros.
+ */
+ if (apr_file_info_get(&finfo, APR_FINFO_MTIME | APR_FINFO_SIZE,
+ db->dirf) == APR_SUCCESS
+ && finfo.mtime != db->mtime)
+ SDBM_INVALIDATE_CACHE(db, finfo);
+ }
+
+ return status;
}
apr_status_t sdbm_unlock(apr_sdbm_t *db)
{
- return apr_file_unlock(db->pagf);
+ return apr_file_unlock(db->dirf);
}
Index: dbm/sdbm/sdbm_private.h
===================================================================
RCS file: /home/cvs/apr-util/dbm/sdbm/sdbm_private.h,v
retrieving revision 1.5
diff -u -r1.5 sdbm_private.h
--- dbm/sdbm/sdbm_private.h 2001/04/30 17:16:23 1.5
+++ dbm/sdbm/sdbm_private.h 2001/05/01 01:17:14
@@ -98,6 +98,7 @@
long dirbno; /* current block in dirbuf */
char dirbuf[DBLKSIZ]; /* directory file block buffer */
apr_status_t status; /* track the specific last error */
+ apr_time_t mtime; /* track the last-modified time */
};
apr_status_t sdbm_lock(apr_sdbm_t *db, int exclusive);
@@ -106,5 +107,15 @@
extern const apr_sdbm_datum_t sdbm_nullitem;
long sdbm_hash(const char *str, int len);
+
+/*
+ * zero the cache
+ */
+#define SDBM_INVALIDATE_CACHE(db, finfo) \
+ do { db->dirbno = (!finfo.size) ? 0 : -1; \
+ db->pagbno = -1; \
+ db->maxbno = finfo.size * BYTESIZ; \
+ db->mtime = finfo.mtime; \
+ } while (0);
#endif /* SDBM_PRIVATE_H */
Re: Why lock pagf?
Posted by Greg Stein <gs...@lyra.org>.
On Mon, Apr 30, 2001 at 08:19:06PM -0500, William A. Rowe, Jr. wrote:
> Greg (and dbm folks),
>
> did you have anything specific in mind when you choose pagf rather
> than dirf?
pagf was opened first, so I added the locking stuff on it (before we
bothered to open dirf).
> I'm asking, since my cache-refresh patch to fstat the
> mtime to determine cache validity aught to correspond to the fstat
> we already need for the dirf. To wrap these all against the same
> file handle would be a good thing.
Nope. pagf can change without a change in dirf. Any mtime-based system you
use will need to use pagf.
> Any reason not to lock against dirf instead?
Not really, you could reorder the two files in prep(). Open one, lock it (if
necessary), then open the other. But see above re: fstat.
> The upshot - realized if we will cache stuff across processes for
> mod_auth_digest, and if users will update passwords in dbm, we don't
> see changing 'trusting' the cache.
The patch below won't work properly.
-1
More on this series in the other thread.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/