acme lock problems

acme lock problems

[Kurt H Maier]

Hello,

IRC user jxy has fixed some problems with p9p acme and would like them
considered for 9front.  See below.  If my mail client fucked up the
text, which is likely, the original is located at 
https://patch-diff.githubusercontent.com/raw/9fans/plan9port/pull/222.patch


khm


 From b14d31ddce2ae16a2251288eb1059d6c25cf7a96 Mon Sep 17 00:00:00 2001
From: Xiao-Yong Jin <xjin@anl.gov>
Date: Tue, 27 Nov 2018 22:34:22 -0600
Subject: [PATCH] acme: fix X, Y pipe command crashing with extra locks

When mouse issues a pipe command, the window that has the mouse is
locked with winlock, then the command is run, and then winunlock.
Runpipe is coping this mechanism by sequentially calling incref,
run (which contains a winclose that does a decref), winunlock, and
finally winlock again.  This is fine for the usual pipe_cmd, but
for X_cmd, all except one window does not have the lock to begin
with.  Acme would crash when such X, Y command were run and winunlock
were called on a window without a prior winlock.

This patch fix the issue by adding extra winunlock/winlock in X_cmd
to deal with existing locks, and applying winlock/winunlock for each
matching window in filelooper.

We remove globalincref and alllocker as they are no longer used.
---
 src/cmd/acme/dat.h  |  1 -
 src/cmd/acme/ecmd.c | 34 ++++++++++++----------------------
 src/cmd/acme/wind.c |  2 --
 3 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/src/cmd/acme/dat.h b/src/cmd/acme/dat.h
index 8a81c97d..02ef3f33 100644
--- a/src/cmd/acme/dat.h
+++ b/src/cmd/acme/dat.h
@@ -508,7 +508,6 @@ enum	/* editing */
 	Collecting
 };
 
-uint		globalincref;
 uint		seq;
 uint		maxtab;	/* size of a tab, in units of the '0' character */
 
diff --git a/src/cmd/acme/ecmd.c b/src/cmd/acme/ecmd.c
index ef92e339..08ab21d3 100644
--- a/src/cmd/acme/ecmd.c
+++ b/src/cmd/acme/ecmd.c
@@ -582,9 +582,11 @@ x_cmd(Text *t, Cmd *cp)
 int
 X_cmd(Text *t, Cmd *cp)
 {
-	USED(t);
-
+	if(t && t->w)
+		winunlock(t->w);
 	filelooper(cp, cp->cmdc=='X');
+	if(t && t->w)
+		winlock(t->w, cp->cmdc);
 	return TRUE;
 }
 
@@ -968,19 +970,11 @@ alllooper(Window *w, void *v)
 	}
 }
 
-void
-alllocker(Window *w, void *v)
-{
-	if(v)
-		incref(&w->ref);
-	else
-		winclose(w);
-}
-
 void
 filelooper(Cmd *cp, int XY)
 {
 	int i;
+	Window *w;
 
 	if(Glooping++)
 		editerror("can't nest %c command", "YX"[XY]);
@@ -993,18 +987,14 @@ filelooper(Cmd *cp, int XY)
 	loopstruct.w = nil;
 	loopstruct.nw = 0;
 	allwindows(alllooper, &loopstruct);
-	/*
-	 * add a ref to all windows to keep safe windows accessed by X
-	 * that would not otherwise have a ref to hold them up during
-	 * the shenanigans.  note this with globalincref so that any
-	 * newly created windows start with an extra reference.
-	 */
-	allwindows(alllocker, (void*)1);
-	globalincref = 1;
-	for(i=0; i<loopstruct.nw; i++)
+	for(i=0; i<loopstruct.nw; i++){
+		w = loopstruct.w[i]->body.w;
+		if(w)
+			winlock(w, "YX"[XY]);
 		cmdexec(&loopstruct.w[i]->body, cp->u.cmd);
-	allwindows(alllocker, (void*)0);
-	globalincref = 0;
+		if(w)
+			winunlock(w);
+	}
 	free(loopstruct.w);
 	loopstruct.w = nil;
 
diff --git a/src/cmd/acme/wind.c b/src/cmd/acme/wind.c
index 19b52f5c..ee8bedc0 100644
--- a/src/cmd/acme/wind.c
+++ b/src/cmd/acme/wind.c
@@ -29,8 +29,6 @@ wininit(Window *w, Window *clone, Rectangle r)
 	w->body.w = w;
 	w->id = ++winid;
 	incref(&w->ref);
-	if(globalincref)
-		incref(&w->ref);
 	w->ctlfid = ~0;
 	w->utflastqid = -1;
 	r1 = r;