changeset 30621:d7875bfbfccb

chg: remove locks

See the previous two patches for the reason. The advantage is a simplified
code base and better throughput when starting multiple servers with multiple
confighashes. The disadvantage is starting multiple servers in parallel with
a single confighash will waste some CPU time, which is probably fine in
common use-cases.

This makes it easier to switch to relative paths to support long unix domain
socket paths.

--- a/contrib/chg/chg.c	Mon Dec 19 22:09:49 2016 +0000
+++ b/contrib/chg/chg.c	Mon Dec 19 22:15:00 2016 +0000
@@ -33,16 +33,13 @@
 	char sockname[UNIX_PATH_MAX];
 	char initsockname[UNIX_PATH_MAX];
 	char redirectsockname[UNIX_PATH_MAX];
-	char lockfile[UNIX_PATH_MAX];
 	size_t argsize;
 	const char **args;
-	int lockfd;
 	int sockdirfd;
 };
 
 static void initcmdserveropts(struct cmdserveropts *opts) {
 	memset(opts, 0, sizeof(struct cmdserveropts));
-	opts->lockfd = -1;
 	opts->sockdirfd = -1;
 }
 
@@ -50,7 +47,6 @@
 	free(opts->args);
 	opts->args = NULL;
 	opts->argsize = 0;
-	assert(opts->lockfd == -1 && "should be closed by unlockcmdserver()");
 	if (opts->sockdirfd >= 0) {
 		close(opts->sockdirfd);
 		opts->sockdirfd = -1;
@@ -157,52 +153,15 @@
 
 	const char *basename = (envsockname) ? envsockname : sockdir;
 	const char *sockfmt = (envsockname) ? "%s" : "%s/server";
-	const char *lockfmt = (envsockname) ? "%s.lock" : "%s/lock";
 	r = snprintf(opts->sockname, sizeof(opts->sockname), sockfmt, basename);
 	if (r < 0 || (size_t)r >= sizeof(opts->sockname))
 		abortmsg("too long TMPDIR or CHGSOCKNAME (r = %d)", r);
-	r = snprintf(opts->lockfile, sizeof(opts->lockfile), lockfmt, basename);
-	if (r < 0 || (size_t)r >= sizeof(opts->lockfile))
-		abortmsg("too long TMPDIR or CHGSOCKNAME (r = %d)", r);
 	r = snprintf(opts->initsockname, sizeof(opts->initsockname),
 			"%s.%u", opts->sockname, (unsigned)getpid());
 	if (r < 0 || (size_t)r >= sizeof(opts->initsockname))
 		abortmsg("too long TMPDIR or CHGSOCKNAME (r = %d)", r);
 }
 
-/*
- * Acquire a file lock that indicates a client is trying to start and connect
- * to a server, before executing a command. The lock is released upon exit or
- * explicit unlock. Will block if the lock is held by another process.
- */
-static void lockcmdserver(struct cmdserveropts *opts)
-{
-	if (opts->lockfd == -1) {
-		opts->lockfd = open(opts->lockfile,
-				    O_RDWR | O_CREAT | O_NOFOLLOW, 0600);
-		if (opts->lockfd == -1)
-			abortmsgerrno("cannot create lock file %s",
-				      opts->lockfile);
-		fsetcloexec(opts->lockfd);
-	}
-	int r = flock(opts->lockfd, LOCK_EX);
-	if (r == -1)
-		abortmsgerrno("cannot acquire lock");
-}
-
-/*
- * Release the file lock held by calling lockcmdserver. Will do nothing if
- * lockcmdserver is not called.
- */
-static void unlockcmdserver(struct cmdserveropts *opts)
-{
-	if (opts->lockfd == -1)
-		return;
-	flock(opts->lockfd, LOCK_UN);
-	close(opts->lockfd);
-	opts->lockfd = -1;
-}
-
 static const char *gethgcmd(void)
 {
 	static const char *hgcmd = NULL;
@@ -308,14 +267,6 @@
 	if (hgc)
 		return hgc;
 
-	lockcmdserver(opts);
-	hgc = hgc_open(sockname);
-	if (hgc) {
-		unlockcmdserver(opts);
-		debugmsg("cmdserver is started by another process");
-		return hgc;
-	}
-
 	/* prevent us from being connected to an outdated server: we were
 	 * told by a server to redirect to opts->redirectsockname and that
 	 * address does not work. we do not want to connect to the server
@@ -334,7 +285,6 @@
 		hgc = retryconnectcmdserver(opts, pid);
 	}
 
-	unlockcmdserver(opts);
 	return hgc;
 }