# HG changeset patch # User Jun Wu # Date 1482185700 0 # Node ID d7875bfbfccbf4ec2a4a9866159929f788a805b4 # Parent 937c52f0670999ee58c9653d8475b192a7e745ff 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. diff -r 937c52f06709 -r d7875bfbfccb contrib/chg/chg.c --- 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; }