Patchwork [3,of,3,V2] chg: remove locks

login
register
mail settings
Submitter Jun Wu
Date Dec. 19, 2016, 10:19 p.m.
Message ID <467f4027b8d0162e3e8f.1482185965@x1c>
Download mbox | patch
Permalink /patch/17970/
State Accepted
Headers show

Comments

Jun Wu - Dec. 19, 2016, 10:19 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1482185700 0
#      Mon Dec 19 22:15:00 2016 +0000
# Node ID 467f4027b8d0162e3e8f09cd0cc0ca627164bce1
# Parent  24534dee38fe515f2c406e18153f623a074148b3
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 467f4027b8d0
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.

Patch

diff --git a/contrib/chg/chg.c b/contrib/chg/chg.c
--- a/contrib/chg/chg.c
+++ b/contrib/chg/chg.c
@@ -34,8 +34,6 @@  struct cmdserveropts {
 	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;
 };
@@ -43,5 +41,4 @@  struct cmdserveropts {
 static void initcmdserveropts(struct cmdserveropts *opts) {
 	memset(opts, 0, sizeof(struct cmdserveropts));
-	opts->lockfd = -1;
 	opts->sockdirfd = -1;
 }
@@ -51,5 +48,4 @@  static void freecmdserveropts(struct cmd
 	opts->args = NULL;
 	opts->argsize = 0;
-	assert(opts->lockfd == -1 && "should be closed by unlockcmdserver()");
 	if (opts->sockdirfd >= 0) {
 		close(opts->sockdirfd);
@@ -158,11 +154,7 @@  static void setcmdserveropts(struct cmds
 	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());
@@ -171,37 +163,4 @@  static void setcmdserveropts(struct cmds
 }
 
-/*
- * 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)
 {
@@ -309,12 +268,4 @@  static hgclient_t *connectcmdserver(stru
 		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
@@ -335,5 +286,4 @@  static hgclient_t *connectcmdserver(stru
 	}
 
-	unlockcmdserver(opts);
 	return hgc;
 }