CVS update: /ccvs/, /ccvs/src/

User: dprice  
Date: 05/05/02 12:16:39

Modified:
 /ccvs/
  ChangeLog, NEWS
 /ccvs/src/
  ChangeLog, cvs.h, lock.c, sanity.sh, tag.c

Log:
 Merge changes from 1.11.x.

File Changes:

Directory: /ccvs/
=================

File [changed]: ChangeLog
Url: https://ccvs.cvshome.org/source/browse/ccvs/ChangeLog?r1=1.1199&r2=1.1200
Delta lines:  +4 -0
-------------------
--- ChangeLog	2 May 2005 14:13:08 -0000	1.1199
+++ ChangeLog	2 May 2005 19:16:35 -0000	1.1200
@@ -1,5 +1,9 @@
 2005-05-02  Derek Price  <[email protected]>
 
+	* NEWS: Note new val-tags locks.
+
+2005-05-02  Derek Price  <[email protected]>
+
 	* NEWS: Note that lock compatibility is now enabled on Windows by
 	default.
 

File [changed]: NEWS
Url: https://ccvs.cvshome.org/source/browse/ccvs/NEWS?r1=1.315&r2=1.316
Delta lines:  +8 -4
-------------------
--- NEWS	2 May 2005 14:13:08 -0000	1.315
+++ NEWS	2 May 2005 19:16:35 -0000	1.316
@@ -14,10 +14,14 @@
 * Thanks to an old report and patch from Stewart Brodie <[email protected]>, a
   potential crash in response to a corrupt RCS file has been fixed.
 
-* CVS now locks the history file before writing to it.  Especially with large
-  repositories, users should no longer see new warnings about corrupt history
-  records when using the `cvs history' command.  Existing corrupt history
-  records will still need to be removed.
+* CVS now locks the history and val-tags files before writing to them.
+  Especially with large repositories, users should no longer see new warnings
+  about corrupt history records when using the `cvs history' command.  Existing
+  corrupt history records will still need to be removed manually.  val-tags
+  corruption should have had less obvious effects, but removing the
+  CVSROOT/val-tags file and allowing a 1.11.21 or later version of CVS to
+  regenerate it may eliminate a few odd behaviors and possibly cause a slight
+  speed up of read transactions in large repositories over time.
 
 Changes from 1.12.11 to 1.12.12:
 ********************************

Directory: /ccvs/src/
=====================

File [changed]: ChangeLog
Url: https://ccvs.cvshome.org/source/browse/ccvs/src/ChangeLog?r1=1.3181&r2=1.3182
Delta lines:  +16 -0
--------------------
--- ChangeLog	30 Apr 2005 16:52:13 -0000	1.3181
+++ ChangeLog	2 May 2005 19:16:35 -0000	1.3182
@@ -1,3 +1,19 @@
+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, val_tags_lock, clear_val_tags_lock): New functions.
+	(history_lock): Use new internal function.
+	* 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-22): Add val-tags lock test.
+
 2005-04-30  Mark D. Baushke  <[email protected]>
 
 	* lock.c (global_readlock, global_writelock, global_history_lock):

File [changed]: cvs.h
Url: https://ccvs.cvshome.org/source/browse/ccvs/src/cvs.h?r1=1.332&r2=1.333
Delta lines:  +6 -1
-------------------
--- cvs.h	30 Apr 2005 03:55:07 -0000	1.332
+++ cvs.h	2 May 2005 19:16:36 -0000	1.333
@@ -193,7 +193,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	CVSPFL		"#cvs.pfl"
 #define	CVSWFL		"#cvs.wfl"
@@ -561,6 +562,10 @@
 int history_lock (const char *);
 void clear_history_lock (void);
 
+/* Get a write lock for the val-tags file.  */
+int val_tags_lock (const char *);
+void clear_val_tags_lock (void);
+
 void Scratch_Entry (List * list, const char *fname);
 void ParseTag (char **tagp, char **datep, int *nonbranchp);
 void WriteTag (const char *dir, const char *tag, const char *date,

File [changed]: lock.c
Url: https://ccvs.cvshome.org/source/browse/ccvs/src/lock.c?r1=1.114&r2=1.115
Delta lines:  +74 -13
---------------------
--- lock.c	30 Apr 2005 16:52:13 -0000	1.114
+++ lock.c	2 May 2005 19:16:36 -0000	1.115
@@ -133,13 +133,17 @@
 static struct lock global_readlock = {NULL, NULL, NULL, CVSLCK, NULL, false};
 static struct lock global_writelock = {NULL, NULL, NULL, CVSLCK, NULL, false};
 
-static struct lock global_history_lock = {NULL, NULL, NULL, CVSHISTLCK,
+static struct lock global_history_lock = {NULL, NULL, NULL, CVSHISTORYLCK,
+					  NULL, false};
+static struct lock global_val_tags_lock = {NULL, NULL, NULL, CVSVALTAGSLCK,
 					  NULL, false};
 #else
 static struct lock global_readlock = {NULL, NULL, CVSLCK, NULL, false};
 static struct lock global_writelock = {NULL, NULL, CVSLCK, NULL, false};
 
-static struct lock global_history_lock = {NULL, NULL, CVSHISTLCK, NULL,
+static struct lock global_history_lock = {NULL, NULL, CVSHISTORYLCK, NULL,
+					  false};
+static struct lock global_val_tags_lock = {NULL, NULL, CVSVALTAGSLCK, NULL,
 					  false};
 #endif /* LOCK_COMPATIBILITY */
 
@@ -385,6 +389,12 @@
     /* clean up simple write locks (if any) */
     if (global_history_lock.repository)
 	remove_lock_files (&global_history_lock, true);
+    SIG_endCrSect();
+
+    SIG_beginCrSect();
+
+    if (global_val_tags_lock.repository)
+	remove_lock_files (&global_val_tags_lock, true);
     /* See note in Lock_Cleanup() below.  */
     SIG_endCrSect();
 }
@@ -1294,21 +1304,42 @@
 
 
 
-/* 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)
-    const char *xrepository;
+static inline int
+internal_lock (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 = Xasprintf ("%s/%s", xrepository,
-						CVSROOTADM);
-    global_history_lock.free_repository = true;
+    assert (!lock->repository);
+    lock->repository = Xasprintf ("%s/%s", xrepository, CVSROOTADM);
+    lock->free_repository = true;
 
     /* 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'",
@@ -1322,10 +1353,40 @@
 
 
 
-/* Remove the history lock, if it exists.
+/* Lock the CVSROOT/history file for write.
+ */
+int
+history_lock (const char *xrepository)
+{
+    return internal_lock (xrepository, L_HISTORY_LOCK);
+}
+
+
+
+/* Remove the CVSROOT/history lock, if it exists.
  */
 void
 clear_history_lock ()
 {
     remove_lock_files (&global_history_lock, true);
 }
+
+
+
+/* Lock the CVSROOT/val-tags file for write.
+ */
+int
+val_tags_lock (const char *xrepository)
+{
+    return internal_lock (xrepository, L_VAL_TAGS_LOCK);
+}
+
+
+
+/* Remove the CVSROOT/val-tags lock, if it exists.
+ */
+void
+clear_val_tags_lock ()
+{
+    remove_lock_files (&global_val_tags_lock, true);
+}

File [changed]: sanity.sh
Url: https://ccvs.cvshome.org/source/browse/ccvs/src/sanity.sh?r1=1.1059&r2=1.1060
Delta lines:  +9 -0
-------------------
--- sanity.sh	30 Apr 2005 03:55:07 -0000	1.1059
+++ sanity.sh	2 May 2005 19:16:36 -0000	1.1060
@@ -22559,6 +22559,15 @@
 $SPROG commit: \[[0-9:]*\] waiting for $username's lock in $CVSROOT_DIRNAME/CVSROOT
 $SPROG 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" \
+"$SPROG update: \[[0-9:]*\] waiting for $username's lock in $CVSROOT_DIRNAME/CVSROOT
+$SPROG update: \[[0-9:]*\] obtained lock in $CVSROOT_DIRNAME/CVSROOT"
+
 	  cd CVSROOT
 	  dotest lockfiles-cleanup-1 "$testcvs -q up -pr1.1 config >config" ""
 	  dotest lockfiles-cleanup-2 "$testcvs -q ci -m config-it" \

File [changed]: tag.c
Url: https://ccvs.cvshome.org/source/browse/ccvs/src/tag.c?r1=1.137&r2=1.138
Delta lines:  +154 -75
----------------------
--- tag.c	17 Mar 2005 22:41:06 -0000	1.137
+++ tag.c	2 May 2005 19:16:36 -0000	1.138
@@ -1410,6 +1410,154 @@
 
 
 
+/* 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 (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 = Xasprintf ("%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 (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 (void *callerdat, const char *dir, const char *repository,
                 const char *update_dir, List *entries)
@@ -1458,10 +1606,6 @@
 tag_check_valid (const char *name, int argc, char **argv, int local, int aflag,
                  char *repository, bool valid)
 {
-    DBM *db;
-    char *valtags_filename;
-    int nowrite = 0;
-    datum mytag, val;
     struct val_args the_val_args;
     struct saved_cwd cwd;
     int which;
@@ -1507,46 +1651,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 = xstrdup (name);
-    mytag.dsize = strlen (name);
-
-    valtags_filename = Xasprintf ("%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)
-    {
-	val = dbm_fetch (db, mytag);
-	if (val.dptr)
-	{
-	    /* The tag is already in val-tags - return valid and don't insert
-	     * it a second time.
-	     */
-	    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;
 
     if (!valid)
     {
@@ -1586,35 +1691,9 @@
 	    error (1, 0, "no such tag `%s'", name);
     }
 
+    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.  */
-    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;
-	}
-    }
-    val.dptr = "y";
-    val.dsize = 1;
-    if (dbm_store (db, mytag, val, DBM_REPLACE) < 0)
-	error (0, errno, "cannot store %s into %s", name,
-	       valtags_filename);
-    dbm_close (db);
-    free (mytag.dptr);
-    free (valtags_filename);
+	add_to_val_tags (name);
 }