CVS update [cvs1-11-x-branch]: /ccvs/src/
[email protected] 2 May 2005 17:07:04 -0000
| Newsgroups | gmane.comp.version-control.cvs.cvs |
|---|---|
| Message-ID | <[email protected]> |
Tag: cvs1-11-x-branch User: dprice Date: 05/05/02 10:07:04 Modified: /ccvs/src/ ChangeLog, cvs.h, lock.c, sanity.sh, tag.c Log: * cvs.h (CVSHISTLCK): Rename macro to... (CVSHISTORYLCK): ...this. (CVSVALTAGSLCK): New macro. (val_tags_lock, clear_val_tags_lock): New functions. * lock.c (global_val_tags_lock): New global. (Lock_Cleanup): Clean up after val-tags lock if necessary. (L_HISTORY_LOCK, L_VAL_TAGS_LOCK): New local macros. (internal_lock, internal_clear_lock, val_tags_lock, clear_val_tags_lock): New functions. (history_lock, clear_history_lock): Use new internal functions. * tag.c (is_in_val_tags, add_to_val_tags): New functions using the write-lock for val-tags and factored from... (tag_check_valid): ...this function. * sanity.sh (lockfiles-21): Add val-tags lock test. File Changes: Directory: /ccvs/src/ ===================== File [changed]: ChangeLog Url: https://ccvs.cvshome.org/source/browse/ccvs/src/ChangeLog?r1=1.2336.2.367&r2=1.2336.2.368 Delta lines: +17 -0 -------------------- --- ChangeLog 28 Apr 2005 20:00:55 -0000 1.2336.2.367 +++ ChangeLog 2 May 2005 17:06:55 -0000 1.2336.2.368 @@ -1,3 +1,20 @@ +2005-05-02 Derek Price <[email protected]> + + * cvs.h (CVSHISTLCK): Rename macro to... + (CVSHISTORYLCK): ...this. + (CVSVALTAGSLCK): New macro. + (val_tags_lock, clear_val_tags_lock): New functions. + * lock.c (global_val_tags_lock): New global. + (Lock_Cleanup): Clean up after val-tags lock if necessary. + (L_HISTORY_LOCK, L_VAL_TAGS_LOCK): New local macros. + (internal_lock, internal_clear_lock, val_tags_lock, + clear_val_tags_lock): New functions. + (history_lock, clear_history_lock): Use new internal functions. + * tag.c (is_in_val_tags, add_to_val_tags): New functions using the + write-lock for val-tags and factored from... + (tag_check_valid): ...this function. + * sanity.sh (lockfiles-21): Add val-tags lock test. + 2005-04-28 Derek Price <[email protected]> * cvs.h (history_lock, clear_history_lock): New protos. File [changed]: cvs.h Url: https://ccvs.cvshome.org/source/browse/ccvs/src/cvs.h?r1=1.235.4.30&r2=1.235.4.31 Delta lines: +6 -1 ------------------- --- cvs.h 28 Apr 2005 20:00:55 -0000 1.235.4.30 +++ cvs.h 2 May 2005 17:06:56 -0000 1.235.4.31 @@ -212,7 +212,8 @@ #define CVSATTIC "Attic" #define CVSLCK "#cvs.lock" -#define CVSHISTLCK "#cvs.history.lock" +#define CVSHISTORYLCK "#cvs.history.lock" +#define CVSVALTAGSLCK "#cvs.val-tags.lock" #define CVSRFL "#cvs.rfl" #define CVSWFL "#cvs.wfl" #define CVSRFLPAT "#cvs.rfl.*" /* wildcard expr to match read locks */ @@ -573,6 +574,10 @@ int history_lock PROTO ((const char *)); void clear_history_lock PROTO ((void)); +/* Get a write lock for the val-tags file. */ +int val_tags_lock PROTO ((const char *)); +void clear_val_tags_lock PROTO ((void)); + /* LockDir setting from CVSROOT/config. */ extern char *lock_dir; File [changed]: lock.c Url: https://ccvs.cvshome.org/source/browse/ccvs/src/lock.c?r1=1.59.4.14&r2=1.59.4.15 Delta lines: +102 -18 ---------------------- --- lock.c 28 Apr 2005 20:00:56 -0000 1.59.4.14 +++ lock.c 2 May 2005 17:06:56 -0000 1.59.4.15 @@ -139,7 +139,8 @@ repository field is NULL if there is no such lock. */ static struct lock global_readlock = {NULL, CVSLCK, NULL}; -static struct lock global_history_lock = {NULL, CVSHISTLCK, NULL}; +static struct lock global_history_lock = {NULL, CVSHISTORYLCK, NULL}; +static struct lock global_val_tags_lock = {NULL, CVSVALTAGSLCK, NULL}; /* List of locks set by lock_tree_for_write. This is redundant with locklist, sort of. */ @@ -328,6 +329,7 @@ } if (global_history_lock.repository) clear_history_lock (); + if (global_val_tags_lock.repository) clear_val_tags_lock (); in_lock_cleanup = 0; } @@ -1059,22 +1061,44 @@ -/* Get a write lock for the history file. Return true on success and false on - * error. +#define L_HISTORY_LOCK 1 +#define L_VAL_TAGS_LOCK 2 + +/* This is the internal implementation behind history_lock & val_tags_lock. It + * gets a write lock for the history or val-tags file. + * + * RETURNS + * true, on success + * false, on error */ -int -history_lock (xrepository) +static int +internal_lock (xrepository, type) const char *xrepository; + int type; { + struct lock *lock; + + switch (type) + { + case L_HISTORY_LOCK: + lock = &global_history_lock; + break; + + case L_VAL_TAGS_LOCK: + lock = &global_val_tags_lock; + break; + + default: + error (1, 0, "internal error: unknown lock type requested"); + } + /* remember what we're locking (for Lock_Cleanup) */ - assert (!global_history_lock.repository); - global_history_lock.repository = xmalloc (strlen (xrepository) - + sizeof (CVSROOTADM) - + 2); - sprintf (global_history_lock.repository, "%s/%s", xrepository, CVSROOTADM); + assert (!lock->repository); + lock->repository = xmalloc (strlen (xrepository) + sizeof (CVSROOTADM) + 2); + sprintf (lock->repository, "%s/%s", xrepository, CVSROOTADM); /* get the lock dir for our own */ - if (set_lock (&global_history_lock, 1) != L_OK) + if (set_lock (lock, 1) != L_OK) { if (!really_quiet) error (0, 0, "failed to obtain history lock in repository `%s'", @@ -1088,18 +1112,78 @@ -/* Remove the history lock, if it exists. +/* This is the internal implementation behind history_lock & val_tags_lock. It + * removes the write lock for the history or val-tags file, when it exists. */ -void -clear_history_lock () +static void +internal_clear_lock (type) + int type; { + struct lock *lock; + + switch (type) + { + case L_HISTORY_LOCK: + lock = &global_history_lock; + break; + + case L_VAL_TAGS_LOCK: + lock = &global_val_tags_lock; + break; + + default: + error (1, 0, "internal error: unknown lock type requested"); + } + SIG_beginCrSect (); - if (global_history_lock.repository) + if (lock->repository) { - free (global_history_lock.repository); - global_history_lock.repository = NULL; + free (lock->repository); + lock->repository = NULL; } SIG_endCrSect (); - clear_lock (&global_history_lock); + clear_lock (lock); +} + + + +/* Lock the CVSROOT/history file for write. + */ +int +history_lock (xrepository) + const char *xrepository; +{ + internal_lock (xrepository, L_HISTORY_LOCK); +} + + + +/* Remove the CVSROOT/history lock, if it exists. + */ +void +clear_history_lock () +{ + internal_clear_lock (L_HISTORY_LOCK); +} + + + +/* Lock the CVSROOT/val-tags file for write. + */ +int +val_tags_lock (xrepository) + const char *xrepository; +{ + internal_lock (xrepository, L_VAL_TAGS_LOCK); +} + + + +/* Remove the CVSROOT/val-tags lock, if it exists. + */ +void +clear_val_tags_lock () +{ + internal_clear_lock (L_VAL_TAGS_LOCK); } File [changed]: sanity.sh Url: https://ccvs.cvshome.org/source/browse/ccvs/src/sanity.sh?r1=1.752.2.169&r2=1.752.2.170 Delta lines: +9 -0 ------------------- --- sanity.sh 28 Apr 2005 20:00:56 -0000 1.752.2.169 +++ sanity.sh 2 May 2005 17:06:56 -0000 1.752.2.170 @@ -20620,6 +20620,15 @@ $PROG commit: \[[0-9:]*\] waiting for $username's lock in $CVSROOT_DIRNAME/CVSROOT $PROG commit: \[[0-9:]*\] obtained lock in $CVSROOT_DIRNAME/CVSROOT" + dotest lockfiles-21 "$testcvs -Q tag newtag first-dir" + + rm $CVSROOT_DIRNAME/CVSROOT/val-tags + mkdir "$TESTDIR/locks/CVSROOT/#cvs.val-tags.lock" + (sleep 5; rmdir "$TESTDIR/locks/CVSROOT/#cvs.val-tags.lock")& + dotest lockfiles-22 "$testcvs -q up -r newtag first-dir" \ +"$PROG update: \[[0-9:]*\] waiting for $username's lock in $CVSROOT_DIRNAME/CVSROOT +$PROG update: \[[0-9:]*\] obtained lock in $CVSROOT_DIRNAME/CVSROOT" + cd CVSROOT echo "# nobody here but us comments" >config dotest lockfiles-cleanup-1 "${testcvs} -q ci -m config-it" \ File [changed]: tag.c Url: https://ccvs.cvshome.org/source/browse/ccvs/src/tag.c?r1=1.100.4.10&r2=1.100.4.11 Delta lines: +161 -79 ---------------------- --- tag.c 16 Mar 2005 19:05:02 -0000 1.100.4.10 +++ tag.c 2 May 2005 17:06:58 -0000 1.100.4.11 @@ -1138,6 +1138,164 @@ +/* This routine determines whether a tag appears in CVSROOT/val-tags. + * + * The val-tags file will be open read-only when IDB is NULL. Since writes to + * val-tags always append to it, the lack of locking is okay. The worst case + * race condition might misinterpret a partially written "foobar" matched, for + * instance, a request for "f", "foo", of "foob". Such a mismatch would be + * caught harmlessly later. + * + * Before CVS adds a tag to val-tags, it will lock val-tags for write and + * verify that the tag is still not present to avoid adding it twice. + * + * NOTES + * This function expects its parent to handle any necessary locking of the + * val-tags file. + * + * INPUTS + * idb When this value is NULL, the val-tags file is opened in + * in read-only mode. When present, the val-tags file is opened + * in read-write mode and the DBM handle is stored in *IDB. + * name The tag to search for. + * + * OUTPUTS + * *idb The val-tags file opened for read/write, or NULL if it couldn't + * be opened. + * + * ERRORS + * Exits with an error message if the val-tags file cannot be opened for + * read (failure to open val-tags read/write is harmless - see below). + * + * RETURNS + * true 1. If NAME exists in val-tags. + * 2. If IDB is non-NULL and val-tags cannot be opened for write. + * This allows callers to ignore the harmless inability to + * update the val-tags cache. + * false If the file could be opened and the tag is not present. + */ +static int is_in_val_tags PROTO((DBM **idb, const char *name)); +static int +is_in_val_tags (idb, name) + DBM **idb; + const char *name; +{ + DBM *db = NULL; + char *valtags_filename; + datum mytag; + int status; + + /* Casting out const should be safe here - input datums are not + * written to by the myndbm functions. + */ + mytag.dptr = (char *)name; + mytag.dsize = strlen (name); + + valtags_filename = xmalloc (strlen (current_parsed_root->directory) + + sizeof CVSROOTADM + + sizeof CVSROOTADM_VALTAGS + 3); + sprintf (valtags_filename, "%s/%s/%s", current_parsed_root->directory, + CVSROOTADM, CVSROOTADM_VALTAGS); + + if (idb) + { + db = dbm_open (valtags_filename, O_RDWR, 0666); + if (!db) + { + mode_t omask; + + if (!existence_error (errno)) + { + error (0, errno, "warning: cannot open %s read/write", + valtags_filename); + *idb = NULL; + return 1; + } + + omask = umask (cvsumask); + db = dbm_open (valtags_filename, O_RDWR | O_CREAT | O_TRUNC, 0666); + umask (omask); + if (!db) + { + error (0, errno, "warning: cannot create %s", + valtags_filename); + *idb = NULL; + return 1; + } + + *idb = db; + return 0; + } + + *idb = db; + } + else + { + db = dbm_open (valtags_filename, O_RDONLY, 0444); + if (!db && !existence_error (errno)) + error (1, errno, "cannot read %s", valtags_filename); + } + + /* If the file merely fails to exist, we just keep going and create + it later if need be. */ + + status = 0; + if (db) + { + datum val; + + val = dbm_fetch (db, mytag); + if (val.dptr != NULL) + /* Found. The tag is valid. */ + status = 1; + + /* FIXME: should check errors somehow (add dbm_error to myndbm.c?). */ + + if (!idb) dbm_close (db); + } + + free (valtags_filename); + return status; +} + + + +/* Add a tag to the CVSROOT/val-tags cache. Establishes a write lock and + * reverifies that the tag does not exist before adding it. + */ +static void add_to_val_tags PROTO((const char *name)); +static void +add_to_val_tags (name) + const char *name; +{ + DBM *db; + datum mytag; + datum value; + + if (noexec) return; + + val_tags_lock (current_parsed_root->directory); + + /* Check for presence again since we have a lock now. */ + if (is_in_val_tags (&db, name)) return; + + /* Casting out const should be safe here - input datums are not + * written to by the myndbm functions. + */ + mytag.dptr = (char *)name; + mytag.dsize = strlen (name); + value.dptr = "y"; + value.dsize = 1; + + if (dbm_store (db, mytag, value, DBM_REPLACE) < 0) + error (0, errno, "failed to store %s into val-tags", name); + dbm_close (db); + + clear_val_tags_lock (); +} + + + static Dtype val_direntproc PROTO ((void *, const char *, const char *, const char *, List *)); @@ -1179,10 +1337,6 @@ int aflag; char *repository; { - DBM *db; - char *valtags_filename; - int nowrite = 0; - datum mytag; struct val_args the_val_args; struct saved_cwd cwd; int which; @@ -1210,49 +1364,7 @@ */ RCS_check_tag (name); - /* FIXME: This routine doesn't seem to do any locking whatsoever - (and it is called from places which don't have locks in place). - If two processes try to write val-tags at the same time, it would - seem like we are in trouble. */ - - mytag.dptr = name; - mytag.dsize = strlen (name); - - valtags_filename = xmalloc (strlen (current_parsed_root->directory) - + sizeof CVSROOTADM - + sizeof CVSROOTADM_VALTAGS + 3); - sprintf (valtags_filename, "%s/%s/%s", current_parsed_root->directory, - CVSROOTADM, CVSROOTADM_VALTAGS); - db = dbm_open (valtags_filename, O_RDWR, 0666); - if (db == NULL) - { - if (!existence_error (errno)) - { - error (0, errno, "warning: cannot open %s read/write", - valtags_filename); - db = dbm_open (valtags_filename, O_RDONLY, 0666); - if (db != NULL) - nowrite = 1; - else if (!existence_error (errno)) - error (1, errno, "cannot read %s", valtags_filename); - } - /* If the file merely fails to exist, we just keep going and create - it later if need be. */ - } - if (db != NULL) - { - datum val; - - val = dbm_fetch (db, mytag); - if (val.dptr != NULL) - { - /* Found. The tag is valid. */ - dbm_close (db); - free (valtags_filename); - return; - } - /* FIXME: should check errors somehow (add dbm_error to myndbm.c?). */ - } + if (is_in_val_tags (NULL, name)) return; /* We didn't find the tag in val-tags, so look through all the RCS files to see whether it exists there. Yes, this is expensive, but there @@ -1297,41 +1409,11 @@ if (!the_val_args.found) error (1, 0, "no such tag %s", name); else - { /* The tags is valid but not mentioned in val-tags. Add it. */ - datum value; + add_to_val_tags (name); +} - if (noexec || nowrite) - { - if (db != NULL) - dbm_close (db); - free (valtags_filename); - return; - } - if (db == NULL) - { - mode_t omask; - omask = umask (cvsumask); - db = dbm_open (valtags_filename, O_RDWR | O_CREAT | O_TRUNC, 0666); - (void)umask (omask); - - if (db == NULL) - { - error (0, errno, "warning: cannot create %s", valtags_filename); - free (valtags_filename); - return; - } - } - value.dptr = "y"; - value.dsize = 1; - if (dbm_store (db, mytag, value, DBM_REPLACE) < 0) - error (0, errno, "cannot store %s into %s", name, - valtags_filename); - dbm_close (db); - } - free (valtags_filename); -} /* * Check whether a join tag is valid. This is just like