git: 9front

Download patch

ref: 6b250a5ca888f3a3cfcb94cbdb3865ae9bcf26e8
parent: 2e4717b4e21ee49ef77271fed6e4ad6b38f532b1
author: rodri <rgl@antares-labs.eu>
date: Sat Sep 6 21:56:06 EDT 2025 

libdraw: locking revamp

Guard the display buffer behind an internal QLock
and provide an RWLock instead for the user.  This
makes it possible to draw in parallel.

The behavior by default (when doing the
display->locking=1 and (un)?lockdisplay dance) is
the same, so existing programs won't break.

Update the manuals and document the special
behavior of menuhit(2) and enter(2) when used in
concurrent programs, since they steal the input
entirely and the user has no way of letting them
know when a resize occurs.  Because of this, when
using screen or _screen, or a custom Screen based
on a volatile image, they may become invisible
while kidnapping the input or end up committing a
despicable use-after-free.


--- a/sys/include/ape/draw.h
+++ b/sys/include/ape/draw.h
@@ -182,8 +182,9 @@
 
 struct Display
 {
-	QLock	qlock;
-	int		locking;	/*program is using lockdisplay */
+	QLock		qlock;
+	RWLock		usrlock;	/* for getwindow(2) and the globals */
+	int		locking;	/* ignored (kept for backwards compatibility) */
 	int		dirno;
 	int		fd;
 	int		reffd;
@@ -201,13 +202,13 @@
 	Image		*transparent;
 	Image		*image;
 	uchar		*buf;
-	int			bufsize;
+	int		bufsize;
 	uchar		*bufp;
 	Font		*defaultfont;
 	Subfont		*defaultsubfont;
 	Image		*windows;
 	Image		*screenimage;
-	int			_isnewdisplay;
+	int		_isnewdisplay;
 };
 
 struct Image
@@ -490,8 +491,16 @@
 extern char*	subfontname(char*, char*, int);
 extern Subfont*	_getsubfont(Display*, char*);
 extern Subfont*	getdefont(Display*);
-extern void		lockdisplay(Display*);
+
+/*
+ * Concurrency
+ */
+extern void	lockdisplay(Display*);
 extern void	unlockdisplay(Display*);
+extern void	rlockdisplay(Display*);
+extern void	runlockdisplay(Display*);
+extern void	_lockdisplay(Display*);
+extern void	_unlockdisplay(Display*);
 
 /*
  * Predefined 
--- a/sys/include/ape/qlock.h
+++ b/sys/include/ape/qlock.h
@@ -26,6 +26,16 @@
 	QLp 	*tail;
 } QLock;
 
+typedef
+struct RWLock
+{
+	Lock	lock;
+	int	readers;	/* number of readers */
+	int	writer;		/* number of writers */
+	QLp	*head;		/* list of waiting processes */
+	QLp	*tail;
+} RWLock;
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -33,6 +43,13 @@
 extern	void	qlock(QLock*);
 extern	void	qunlock(QLock*);
 extern	int	canqlock(QLock*);
+
+extern	void	rlock(RWLock*);
+extern	void	runlock(RWLock*);
+extern	int	canrlock(RWLock*);
+extern	void	wlock(RWLock*);
+extern	void	wunlock(RWLock*);
+extern	int	canwlock(RWLock*);
 
 #ifdef __cplusplus
 }
--- a/sys/include/draw.h
+++ b/sys/include/draw.h
@@ -175,7 +175,8 @@
 struct Display
 {
 	QLock		qlock;
-	int		locking;	/*program is using lockdisplay */
+	RWLock		usrlock;	/* for getwindow(2) and the globals */
+	int		locking;	/* ignored (kept for backwards compatibility) */
 	int		dirno;
 	int		fd;
 	int		reffd;
@@ -484,8 +485,16 @@
 extern char*	subfontname(char*, char*, int);
 extern Subfont*	_getsubfont(Display*, char*);
 extern Subfont*	getdefont(Display*);
-extern void		lockdisplay(Display*);
+
+/*
+ * Concurrency
+ */
+extern void	lockdisplay(Display*);
 extern void	unlockdisplay(Display*);
+extern void	rlockdisplay(Display*);
+extern void	runlockdisplay(Display*);
+extern void	_lockdisplay(Display*);
+extern void	_unlockdisplay(Display*);
 
 /*
  * Predefined 
--- a/sys/man/2/graphics
+++ b/sys/man/2/graphics
@@ -52,6 +52,12 @@
 void	unlockdisplay(Display *d)
 .PP
 .B
+void	rlockdisplay(Display *d)
+.PP
+.B
+void	runlockdisplay(Display *d)
+.PP
+.B
 int	getwindow(Display *d, int ref)
 .PP
 .B
@@ -417,18 +423,18 @@
 .I initdraw
 calls them as needed.
 .PP
-The data structures associated with the display must be protected in a multi-process program,
-because they assume only one process will be using them at a time.
-Multi-process programs should set
-.B display->locking
-to
-.BR 1 ,
-to notify the library to use a locking protocol for its own accesses,
-and call
+When drawing concurrently programs must call
 .I lockdisplay
 and
 .I unlockdisplay
-around any calls to the graphics library that will cause messages to be sent to the display device.
+around any calls to
+.I getwindow
+or any other procedures that could change its global variables (read
+below), and
+.I rlockdisplay
+and
+.I runlockdisplay
+around any access to such variables.
 .I Initdraw
 and
 .I geninitdraw
@@ -592,7 +598,7 @@
 	sysfatal("can't open $wsys: %r");
 
 /* mount creates window; see \f2rio\fP(4) */
-if(mount(fd, -1, "/tmp", MREPL, "new -dx 300-dy 200") < 0)
+if(mount(fd, -1, "/tmp", MREPL, "new -dx 300 -dy 200") < 0)
 	sysfatal("can't mount new window: %r");
 if(gengetwindow(display, "/tmp/winname",
    &screen2, &_screen2, Refnone) < 0)
@@ -636,3 +642,6 @@
 .B white
 and
 .BR black .
+.br
+.B Display->locking
+is now ignored, but is kept for backwards compatibility.
--- a/sys/man/2/mouse
+++ b/sys/man/2/mouse
@@ -263,3 +263,21 @@
 .IR event (2),
 .IR keyboard (2),
 .IR thread (2).
+.SH BUGS
+.I Menuhit
+and
+.I enter
+are not re-entrant and will break when used concurrently with an
+independent window whose
+.B Screen
+changes during their execution (as is the case when receiving a resize
+event while using
+.BR _screen ).
+In such cases they must be recreated after acquiring the new screen.
+When used with a
+.B Screen
+that has an independent backing window (i.e. not
+.BR display->image / screen ),
+this backing window must be drawn periodically to the
+.BR display->image / screen
+to be able to see the changes induced by the input.
--- a/sys/src/ape/lib/ap/plan9/qlock.c
+++ b/sys/src/ape/lib/ap/plan9/qlock.c
@@ -22,6 +22,8 @@
 enum
 {
 	Queuing,
+	QueuingR,
+	QueuingW,
 };
 
 /* find a free shared memory location to queue ourselves in */
@@ -107,4 +109,170 @@
 	}
 	unlock(&q->lock);
 	return 0;
+}
+
+void
+rlock(RWLock *q)
+{
+	QLp *p, *mp;
+
+	lock(&q->lock);
+	if(q->writer == 0 && q->head == nil){
+		/* no writer, go for it */
+		q->readers++;
+		unlock(&q->lock);
+		return;
+	}
+
+	mp = getqlp();
+	p = q->tail;
+	if(p == nil)
+		q->head = mp;
+	else
+		p->next = mp;
+	q->tail = mp;
+	mp->state = QueuingR;
+	unlock(&q->lock);
+
+	/* wait in kernel */
+	while((*_rendezvousp)(mp, (void*)1) == (void*)~0)
+		;
+	mp->inuse = 0;
+}
+
+int
+canrlock(RWLock *q)
+{
+	lock(&q->lock);
+	if (q->writer == 0 && q->head == nil) {
+		/* no writer; go for it */
+		q->readers++;
+		unlock(&q->lock);
+		return 1;
+	}
+	unlock(&q->lock);
+	return 0;
+}
+
+void
+runlock(RWLock *q)
+{
+	QLp *p;
+
+	lock(&q->lock);
+	if(q->readers <= 0)
+		abort();
+	p = q->head;
+	if(--(q->readers) > 0 || p == nil){
+		unlock(&q->lock);
+		return;
+	}
+
+	/* start waiting writer */
+	if(p->state != QueuingW)
+		abort();
+	q->head = p->next;
+	if(q->head == nil)
+		q->tail = nil;
+	q->writer = 1;
+	unlock(&q->lock);
+
+	/* wakeup waiter */
+	while((*_rendezvousp)(p, 0) == (void*)~0)
+		;
+}
+
+void
+wlock(RWLock *q)
+{
+	QLp *p, *mp;
+
+	lock(&q->lock);
+	if(q->readers == 0 && q->writer == 0){
+		/* noone waiting, go for it */
+		q->writer = 1;
+		unlock(&q->lock);
+		return;
+	}
+
+	/* wait */
+	mp = getqlp();
+	p = q->tail;
+	if(p == nil)
+		q->head = mp;
+	else
+		p->next = mp;
+	q->tail = mp;
+	mp->state = QueuingW;
+	unlock(&q->lock);
+
+	/* wait in kernel */
+	while((*_rendezvousp)(mp, (void*)1) == (void*)~0)
+		;
+	mp->inuse = 0;
+}
+
+int
+canwlock(RWLock *q)
+{
+	lock(&q->lock);
+	if (q->readers == 0 && q->writer == 0) {
+		/* no one waiting; go for it */
+		q->writer = 1;
+		unlock(&q->lock);
+		return 1;
+	}
+	unlock(&q->lock);
+	return 0;
+}
+
+void
+wunlock(RWLock *q)
+{
+	QLp *p, *x;
+
+	lock(&q->lock);
+	if(q->writer == 0)
+		abort();
+	p = q->head;
+	if(p == nil){
+		q->writer = 0;
+		unlock(&q->lock);
+		return;
+	}
+	if(p->state == QueuingW){
+		/* start waiting writer */
+		q->head = p->next;
+		if(q->head == nil)
+			q->tail = nil;
+		unlock(&q->lock);
+		while((*_rendezvousp)(p, 0) == (void*)~0)
+			;
+		return;
+	}
+	if(p->state != QueuingR)
+		abort();
+
+	/* collect waiting readers */
+	q->readers = 1;
+	for(x = p->next; x != nil && x->state == QueuingR; x = x->next){
+		q->readers++;
+		p = x;
+	}
+	p->next = nil;
+	p = q->head;
+
+	/* queue remaining writers */
+	q->head = x;
+	if(x == nil)
+		q->tail = nil;
+	q->writer = 0;
+	unlock(&q->lock);
+
+	/* wakeup waiting readers */
+	for(; p != nil; p = x){
+		x = p->next;
+		while((*_rendezvousp)(p, 0) == (void*)~0)
+			;
+	}
 }
--- a/sys/src/libdraw/alloc.c
+++ b/sys/src/libdraw/alloc.c
@@ -47,9 +47,12 @@
 		return nil;
 	}
 
+	_lockdisplay(d);
 	a = bufimage(d, 1+4+4+1+4+1+4*4+4*4+4);
-	if(a == nil)
+	if(a == nil){
+		_unlockdisplay(d);
 		goto Error;
+	}
 	d->imageid++;
 	id = d->imageid;
 	a[0] = 'b';
@@ -72,6 +75,7 @@
 	BPLONG(a+39, clipr.max.x);
 	BPLONG(a+43, clipr.max.y);
 	BPLONG(a+47, col);
+	_unlockdisplay(d);
 
 	if(ai != nil)
 		i = ai;
@@ -78,12 +82,15 @@
 	else{
 		i = malloc(sizeof(Image));
 		if(i == nil){
+			_lockdisplay(d);
 			a = bufimage(d, 1+4);
 			if(a != nil){
 				a[0] = 'f';
 				BPLONG(a+1, id);
+				_unlockdisplay(d);
 				flushimage(d, 0);
-			}
+			}else
+				_unlockdisplay(d);
 			goto Error;
 		}
 	}
@@ -124,9 +131,12 @@
 	}
 	/* flush pending data so we don't get error allocating the image */
 	flushimage(d, 0);
+	_lockdisplay(d);
 	a = bufimage(d, 1+4+1+n);
-	if(a == nil)
+	if(a == nil){
+		_unlockdisplay(d);
 		goto Error;
+	}
 	d->imageid++;
 	id = d->imageid;
 	a[0] = 'n';
@@ -133,22 +143,30 @@
 	BPLONG(a+1, id);
 	a[5] = n;
 	memmove(a+6, name, n);
+	_unlockdisplay(d);
 	if(flushimage(d, 0) < 0)
 		goto Error;
 
-	if(pread(d->ctlfd, buf, sizeof buf, 0) < 12*12)
+	_lockdisplay(d);
+	if(pread(d->ctlfd, buf, sizeof buf, 0) < 12*12){
+		_unlockdisplay(d);
 		goto Error;
+	}
 	buf[12*12] = '\0';
+	_unlockdisplay(d);
 
 	i = malloc(sizeof(Image));
 	if(i == nil){
 	Error1:
+		_lockdisplay(d);
 		a = bufimage(d, 1+4);
 		if(a != nil){
 			a[0] = 'f';
 			BPLONG(a+1, id);
+			_unlockdisplay(d);
 			flushimage(d, 0);
-		}
+		}else
+			_unlockdisplay(d);
 		goto Error;
 	}
 	i->display = d;
@@ -180,14 +198,18 @@
 	int n;
 
 	n = strlen(name);
+	_lockdisplay(i->display);
 	a = bufimage(i->display, 1+4+1+1+n);
-	if(a == nil)
+	if(a == nil){
+		_unlockdisplay(i->display);
 		return 0;
+	}
 	a[0] = 'N';
 	BPLONG(a+1, i->id);
 	a[5] = in;
 	a[6] = n;
 	memmove(a+7, name, n);
+	_unlockdisplay(i->display);
 	if(flushimage(i->display, 0) < 0)
 		return 0;
 	return 1;
@@ -198,29 +220,31 @@
 {
 	uchar *a;
 	Display *d;
-	Image *w;
+	Image **w;
 
 	if(i == nil || i->display == nil)
 		return 0;
 	d = i->display;
-	if(i->screen != nil){
-		w = d->windows;
-		if(w == i)
-			d->windows = i->next;
-		else
-			while(w != nil){
-				if(w->next == i){
-					w->next = i->next;
-					break;
-				}
-				w = w->next;
-			}
-	}
+	_lockdisplay(d);
 	a = bufimage(d, 1+4);
-	if(a == nil)
+	if(a == nil){
+		_unlockdisplay(d);
 		return -1;
+	}
 	a[0] = 'f';
 	BPLONG(a+1, i->id);
+
+	if(i->screen != nil){
+		w = &d->windows;
+		while(*w != nil){
+			if(*w == i){
+				*w = i->next;
+				break;
+			}
+			w = &(*w)->next;
+		}
+	}
+	_unlockdisplay(d);
 	return 0;
 }
 
--- a/sys/src/libdraw/cloadimage.c
+++ b/sys/src/libdraw/cloadimage.c
@@ -30,9 +30,12 @@
 			werrstr("cloadimage: bad count %d", nb);
 			return -1;
 		}
+		_lockdisplay(i->display);
 		a = bufimage(i->display, 21+nb);
-		if(a == nil)
+		if(a == nil){
+			_unlockdisplay(i->display);
 			return -1;
+		}
 		a[0] = 'Y';
 		BPLONG(a+1, i->id);
 		BPLONG(a+5, r.min.x);
@@ -40,6 +43,7 @@
 		BPLONG(a+13, r.max.x);
 		BPLONG(a+17, maxy);
 		memmove(a+21, data, nb);
+		_unlockdisplay(i->display);
 		miny = maxy;
 		data += nb;
 		ndata += nb;
--- a/sys/src/libdraw/creadimage.c
+++ b/sys/src/libdraw/creadimage.c
@@ -3,7 +3,7 @@
 #include <draw.h>
 
 Image *
-creadimage(Display *d, int fd, int dolock)
+creadimage(Display *d, int fd, int)
 {
 	char hdr[5*12+1];
 	Rectangle r;
@@ -55,11 +55,7 @@
 	}
 
 	if(d != nil){
-		if(dolock)
-			lockdisplay(d);
 		i = allocimage(d, r, chan, 0, 0);
-		if(dolock)
-			unlockdisplay(d);
 		if(i == nil)
 			return nil;
 	}else{
@@ -76,12 +72,7 @@
 	while(miny != r.max.y){
 		if(readn(fd, hdr, 2*12) != 2*12){
 		Errout:
-			if(dolock)
-				lockdisplay(d);
-		Erroutlock:
 			freeimage(i);
-			if(dolock)
-				unlockdisplay(d);
 			free(buf);
 			return nil;
 		}
@@ -98,11 +89,12 @@
 		if(readn(fd, buf, nb)!=nb)
 			goto Errout;
 		if(d != nil){
-			if(dolock)
-				lockdisplay(d);
+			_lockdisplay(d);
 			a = bufimage(i->display, 21+nb);
-			if(a == nil)
-				goto Erroutlock;
+			if(a == nil){
+				_unlockdisplay(d);
+				goto Errout;
+			}
 			a[0] = 'Y';
 			BPLONG(a+1, i->id);
 			BPLONG(a+5, r.min.x);
@@ -112,8 +104,7 @@
 			if(!new)	/* old image: flip the data bits */
 				_twiddlecompressed(buf, nb);
 			memmove(a+21, buf, nb);
-			if(dolock)
-				unlockdisplay(d);
+			_unlockdisplay(d);
 		}
 		miny = maxy;
 	}
--- a/sys/src/libdraw/debug.c
+++ b/sys/src/libdraw/debug.c
@@ -6,11 +6,15 @@
 drawsetdebug(int v)
 {
 	uchar *a;
+
+	_lockdisplay(display);
 	a = bufimage(display, 1+1);
 	if(a == nil){
+		_unlockdisplay(display);
 		fprint(2, "drawsetdebug: %r\n");
 		return;
 	}
 	a[0] = 'D';
 	a[1] = v;
+	_unlockdisplay(display);
 }
--- a/sys/src/libdraw/draw.c
+++ b/sys/src/libdraw/draw.c
@@ -8,11 +8,15 @@
 	uchar *a;
 
 	if(op != SoverD){
+		_lockdisplay(d);
 		a = bufimage(d, 1+1);
-		if(a == nil)
+		if(a == nil){
+			_unlockdisplay(d);
 			return;
+		}
 		a[0] = 'O';
 		a[1] = op;
+		_unlockdisplay(d);
 	}
 }
 		
@@ -23,9 +27,12 @@
 
 	_setdrawop(dst->display, op);
 
+	_lockdisplay(dst->display);
 	a = bufimage(dst->display, 1+4+4+4+4*4+2*4+2*4);
-	if(a == nil)
+	if(a == nil){
+		_unlockdisplay(dst->display);
 		return;
+	}
 	if(src == nil)
 		src = dst->display->black;
 	if(mask == nil)
@@ -42,6 +49,7 @@
 	BPLONG(a+33, p0->y);
 	BPLONG(a+37, p1->x);
 	BPLONG(a+41, p1->y);
+	_unlockdisplay(dst->display);
 }
 
 void
--- a/sys/src/libdraw/ellipse.c
+++ b/sys/src/libdraw/ellipse.c
@@ -10,8 +10,10 @@
 
 	_setdrawop(dst->display, op);
 
+	_lockdisplay(dst->display);
 	a = bufimage(dst->display, 1+4+4+2*4+4+4+4+2*4+2*4);
 	if(a == nil){
+		_unlockdisplay(dst->display);
 		fprint(2, "image ellipse: %r\n");
 		return;
 	}
@@ -27,6 +29,7 @@
 	BPLONG(a+33, sp->y);
 	BPLONG(a+37, alpha);
 	BPLONG(a+41, phi);
+	_unlockdisplay(dst->display);
 }
 
 void
--- a/sys/src/libdraw/emenuhit.c
+++ b/sys/src/libdraw/emenuhit.c
@@ -27,10 +27,11 @@
 menucolors(void)
 {
 	/* Main tone is greenish, with negative selection */
+	menutxt = allocimage(display, Rect(0,0,1,1), screen->chan, 1, DDarkgreen);
 	back = allocimagemix(display, DPalegreen, DWhite);
-	high = allocimage(display, Rect(0,0,1,1), CMAP8, 1, DDarkgreen);	/* dark green */
-	bord = allocimage(display, Rect(0,0,1,1), CMAP8, 1, DMedgreen);	/* not as dark green */
-	if(back==nil || high==nil || bord==nil)
+	high = allocimage(display, Rect(0,0,1,1), screen->chan, 1, DDarkgreen);	/* dark green */
+	bord = allocimage(display, Rect(0,0,1,1), screen->chan, 1, DMedgreen);	/* not as dark green */
+	if(menutxt==nil || back==nil || high==nil || bord==nil)
 		goto Error;
 	text = display->black;
 	htext = back;
@@ -37,9 +38,11 @@
 	return;
 
     Error:
+	freeimage(menutxt);
 	freeimage(back);
 	freeimage(high);
 	freeimage(bord);
+	menutxt = display->black;
 	back = display->white;
 	high = display->black;
 	bord = display->black;
@@ -108,10 +111,10 @@
 	int i;
 
 	paintitem(menu, textr, off, lasti, 1, save, nil);
-	flushimage(display, 1);	/* in case display->locking is set */
+	flushimage(display, 1);
 	*m = emouse();
 	while(m->buttons & (1<<(but-1))){
-		flushimage(display, 1);	/* in case display->locking is set */
+		flushimage(display, 1);
 		*m = emouse();
 		i = menusel(textr, m->xy);
 		if(i != -1 && i == lasti)
@@ -148,10 +151,7 @@
 	if(r.max.y < r.min.y+2)
 		r.max.y = r.min.y+2;
 	border(screen, r, 1, bord, ZP);
-	if(menutxt == 0)
-		menutxt = allocimage(display, Rect(0, 0, 1, 1), CMAP8, 1, DDarkgreen);
-	if(menutxt)
-		draw(screen, insetrect(r, 1), menutxt, nil, ZP);
+	draw(screen, insetrect(r, 1), menutxt, nil, ZP);
 }
 
 int
@@ -252,7 +252,7 @@
 					menuscrollpaint(scrollr, off, nitem, nitemdrawn);
 				}
 			}
-			flushimage(display, 1);	/* in case display->locking is set */
+			flushimage(display, 1);
 			*m = emouse();
 		}
 	}
--- a/sys/src/libdraw/event.c
+++ b/sys/src/libdraw/event.c
@@ -261,19 +261,8 @@
 	int i, n;
 	uchar ebuf[EMAXMSG+1];
 
-	/* avoid generating a message if there's nothing to show. */
-	/* this test isn't perfect, though; could do flushimage(display, 0) then call extract */
-	/* also: make sure we don't interfere if we're multiprocessing the display */
-	if(display->locking){
-		/* if locking is being done by program, this means it can't depend on automatic flush in emouse() etc. */
-		if(canqlock(&display->qlock)){
-			if(display->bufp > display->buf)
-				flushimage(display, 1);
-			unlockdisplay(display);
-		}
-	}else
-		if(display->bufp > display->buf)
-			flushimage(display, 1);
+	if(display->bufp > display->buf)
+		flushimage(display, 1);
 loop:
 	if((n=read(epipe[0], ebuf, EMAXMSG+1)) < 0
 	|| ebuf[0] >= MAXSLAVE)
--- a/sys/src/libdraw/font.c
+++ b/sys/src/libdraw/font.c
@@ -284,9 +284,12 @@
 	c->left = fi->left;
 	if(f->display == nil)
 		return 1;
+	_lockdisplay(f->display);
 	b = bufimage(f->display, 37);
-	if(b == nil)
+	if(b == nil){
+		_unlockdisplay(f->display);
 		return 0;
+	}
 	top = fi->top + (f->ascent-subf->f->ascent);
 	bottom = fi->bottom + (f->ascent-subf->f->ascent);
 	b[0] = 'l';
@@ -301,6 +304,7 @@
 	BPLONG(b+31, fi->top);
 	b[35] = fi->left;
 	b[36] = fi->width;
+	_unlockdisplay(f->display);
 	return 1;
 }
 
@@ -329,8 +333,10 @@
 		fprint(2, "font cache resize failed: %r\n");
 		goto Return;
 	}
+	_lockdisplay(d);
 	b = bufimage(d, 1+4+4+1);
 	if(b == nil){
+		_unlockdisplay(d);
 		freeimage(new);
 		goto Return;
 	}
@@ -338,6 +344,7 @@
 	BPLONG(b+1, new->id);
 	BPLONG(b+5, ncache);
 	b[9] = f->ascent;
+	_unlockdisplay(d);
 	freeimage(f->cacheimage);
 	f->cacheimage = new;
     Nodisplay:
--- a/sys/src/libdraw/getsubfont.c
+++ b/sys/src/libdraw/getsubfont.c
@@ -9,32 +9,19 @@
 Subfont*
 _getsubfont(Display *d, char *name)
 {
-	int dolock, fd;
+	int fd;
 	Subfont *f;
 
-	/*
-	 * unlock display so i/o happens with display released, unless
-	 * user is doing his own locking, in which case this could break things.
-	 * _getsubfont is called only from string.c and stringwidth.c,
-	 * which are known to be safe to have this done.
-	 */
-	dolock = d != nil && d->locking == 0;
-	if(dolock)
-		unlockdisplay(d);
-
 	fd = open(name, OREAD|OCEXEC);
 	if(fd < 0) {
 		fprint(2, "getsubfont: can't open %s: %r\n", name);
 		f = nil;
 	} else {
-		f = readsubfont(d, name, fd, dolock);
+		f = readsubfont(d, name, fd, 0);
 		if(f == nil)
 			fprint(2, "getsubfont: can't read %s: %r\n", name);
 		close(fd);
 	}
-
-	if(dolock)
-		lockdisplay(d);
 
 	return f;
 }
--- a/sys/src/libdraw/init.c
+++ b/sys/src/libdraw/init.c
@@ -9,8 +9,6 @@
 static char deffontname[] = "*default*";
 Screen	*_screen;
 
-int	debuglockdisplay = 0;
-
 static void _closedisplay(Display*, int);
 
 /* note handler */
@@ -300,7 +298,7 @@
 	disp->error = error;
 	disp->windir = t;
 	disp->devdir = strdup(dev);
-	qlock(&disp->qlock);
+	wlock(&disp->usrlock);
 	disp->white = allocimage(disp, Rect(0, 0, 1, 1), GREY1, 1, DWhite);
 	disp->black = allocimage(disp, Rect(0, 0, 1, 1), GREY1, 1, DBlack);
 	if(disp->white == nil || disp->black == nil){
@@ -324,7 +322,6 @@
 }
 
 /*
- * Call with d unlocked.
  * Note that disp->defaultfont and defaultsubfont are not freed here.
  */
 void
@@ -369,7 +366,6 @@
 	close(disp->ctlfd);
 	/* should cause refresh slave to shut down */
 	close(disp->reffd);
-	qunlock(&disp->qlock);
 	free(disp);
 }
 
@@ -376,17 +372,36 @@
 void
 lockdisplay(Display *disp)
 {
-	if(debuglockdisplay){
-		/* avoid busy looping; it's rare we collide anyway */
-		while(!canqlock(&disp->qlock))
-			sleep(1000);
-	}else
-		qlock(&disp->qlock);
+	wlock(&disp->usrlock);
 }
 
 void
 unlockdisplay(Display *disp)
 {
+	wunlock(&disp->usrlock);
+}
+
+void
+rlockdisplay(Display *disp)
+{
+	rlock(&disp->usrlock);
+}
+
+void
+runlockdisplay(Display *disp)
+{
+	runlock(&disp->usrlock);
+}
+
+void
+_lockdisplay(Display *disp)
+{
+	qlock(&disp->qlock);
+}
+
+void
+_unlockdisplay(Display *disp)
+{
 	qunlock(&disp->qlock);
 }
 
@@ -425,8 +440,11 @@
 int
 flushimage(Display *d, int visible)
 {
+	int rc;
+
 	if(d == nil)
 		return 0;
+	_lockdisplay(d);
 	if(visible){
 		*d->bufp++ = 'v';	/* five bytes always reserved for this */
 		if(d->_isnewdisplay){
@@ -434,7 +452,9 @@
 			d->bufp += 4;
 		}
 	}
-	return doflush(d);
+	rc = doflush(d);
+	_unlockdisplay(d);
+	return rc;
 }
 
 uchar*
@@ -453,4 +473,3 @@
 	d->bufp += n;
 	return p;
 }
-
--- a/sys/src/libdraw/line.c
+++ b/sys/src/libdraw/line.c
@@ -15,8 +15,10 @@
 
 	_setdrawop(dst->display, op);
 
+	_lockdisplay(dst->display);
 	a = bufimage(dst->display, 1+4+2*4+2*4+4+4+4+4+2*4);
 	if(a == nil){
+		_unlockdisplay(dst->display);
 		fprint(2, "image line: %r\n");
 		return;
 	}
@@ -32,4 +34,5 @@
 	BPLONG(a+33, src->id);
 	BPLONG(a+37, sp.x);
 	BPLONG(a+41, sp.y);
+	_unlockdisplay(dst->display);
 }
--- a/sys/src/libdraw/loadimage.c
+++ b/sys/src/libdraw/loadimage.c
@@ -36,8 +36,10 @@
 				return -1;
 		} else
 			n = dy*bpl;
+		_lockdisplay(i->display);
 		a = bufimage(i->display, 21+n);
 		if(a == nil){
+			_unlockdisplay(i->display);
 			werrstr("loadimage: %r");
 			return -1;
 		}
@@ -48,6 +50,7 @@
 		BPLONG(a+13, r.min.x+dx);
 		BPLONG(a+17, r.min.y+dy);
 		memmove(a+21, data, n);
+		_unlockdisplay(i->display);
 		ndata += dy*bpl;
 		data += dy*bpl;
 		r.min.y += dy;
--- a/sys/src/libdraw/menuhit.c
+++ b/sys/src/libdraw/menuhit.c
@@ -28,10 +28,11 @@
 menucolors(void)
 {
 	/* Main tone is greenish, with negative selection */
+	menutxt = allocimage(display, Rect(0,0,1,1), screen->chan, 1, DDarkgreen);
 	back = allocimagemix(display, DPalegreen, DWhite);
 	high = allocimage(display, Rect(0,0,1,1), screen->chan, 1, DDarkgreen);	/* dark green */
 	bord = allocimage(display, Rect(0,0,1,1), screen->chan, 1, DMedgreen);	/* not as dark green */
-	if(back==nil || high==nil || bord==nil)
+	if(menutxt==nil || back==nil || high==nil || bord==nil)
 		goto Error;
 	text = display->black;
 	htext = back;
@@ -38,9 +39,11 @@
 	return;
 
     Error:
+	freeimage(menutxt);
 	freeimage(back);
 	freeimage(high);
 	freeimage(bord);
+	menutxt = display->black;
 	back = display->white;
 	high = display->black;
 	bord = display->black;
@@ -145,10 +148,7 @@
 	if(r.max.y < r.min.y+2)
 		r.max.y = r.min.y+2;
 	border(m, r, 1, bord, ZP);
-	if(menutxt == 0)
-		menutxt = allocimage(display, Rect(0, 0, 1, 1), screen->chan, 1, DDarkgreen);	/* border color; BUG? */
-	if(menutxt)
-		draw(m, insetrect(r, 1), menutxt, nil, ZP);
+	draw(m, insetrect(r, 1), menutxt, nil, ZP);
 }
 
 int
--- a/sys/src/libdraw/poly.c
+++ b/sys/src/libdraw/poly.c
@@ -43,8 +43,10 @@
 
 	_setdrawop(dst->display, op);
 
+	_lockdisplay(dst->display);
 	a = bufimage(dst->display, 1+4+2+4+4+4+4+2*4+(u-t));
 	if(a == nil){
+		_unlockdisplay(dst->display);
 		free(t);
 		fprint(2, "image poly: %r\n");
 		return;
@@ -59,6 +61,7 @@
 	BPLONG(a+23, sp->x);
 	BPLONG(a+27, sp->y);
 	memmove(a+31, t, u-t);
+	_unlockdisplay(dst->display);
 	free(t);
 }
 
--- a/sys/src/libdraw/readimage.c
+++ b/sys/src/libdraw/readimage.c
@@ -3,7 +3,7 @@
 #include <draw.h>
 
 Image*
-readimage(Display *d, int fd, int dolock)
+readimage(Display *d, int fd, int)
 {
 	char hdr[5*12+1];
 	int dy;
@@ -20,7 +20,7 @@
 	if(readn(fd, hdr, 11) != 11)
 		return nil;
 	if(memcmp(hdr, "compressed\n", 11) == 0){
-		if(i = creadimage(d, fd, dolock))
+		if(i = creadimage(d, fd, 0))
 			goto Done;
 		return nil;
 	}
@@ -82,11 +82,7 @@
 	if(l > chunk)
 		chunk = l;
 	if(d != nil){
-		if(dolock)
-			lockdisplay(d);
 		i = allocimage(d, r, chan, 0, -1);
-		if(dolock)
-			unlockdisplay(d);
 		if(i == nil)
 			return nil;
 	}else{
@@ -106,12 +102,7 @@
 		if(m != n){
 			werrstr("readimage: read count %d not %d: %r", m, n);
    Err:
-			if(dolock)
-				lockdisplay(d);
-   Err1:
  			freeimage(i);
-			if(dolock)
-				unlockdisplay(d);
 			free(tmp);
 			return nil;
 		}
@@ -120,12 +111,8 @@
 				tmp[j] ^= 0xFF;
 
 		if(d != nil){
-			if(dolock)
-				lockdisplay(d);
 			if(loadimage(i, Rect(r.min.x, miny, r.max.x, miny+dy), tmp, chunk) <= 0)
-				goto Err1;
-			if(dolock)
-				unlockdisplay(d);
+				goto Err;
 		}
 		miny += dy;
 	}
--- a/sys/src/libdraw/readsubfont.c
+++ b/sys/src/libdraw/readsubfont.c
@@ -3,7 +3,7 @@
 #include <draw.h>
 
 Subfont*
-readsubfonti(Display*d, char *name, int fd, Image *ai, int dolock)
+readsubfonti(Display*d, char *name, int fd, Image *ai, int)
 {
 	char hdr[3*12+4+1];
 	int n;
@@ -14,7 +14,7 @@
 
 	i = ai;
 	if(i == nil){
-		i = readimage(d, fd, dolock);
+		i = readimage(d, fd, 0);
 		if(i == nil)
 			return nil;
 	}
@@ -39,11 +39,7 @@
 	if(fc == nil)
 		goto Err;
 	_unpackinfo(fc, p, n);
-	if(dolock)
-		lockdisplay(d);
 	f = allocsubfont(name, n, atoi(hdr+12), atoi(hdr+24), fc, i);
-	if(dolock)
-		unlockdisplay(d);
 	if(f == nil){
 		free(fc);
 		goto Err;
@@ -58,7 +54,7 @@
 }
 
 Subfont*
-readsubfont(Display *d, char *name, int fd, int dolock)
+readsubfont(Display *d, char *name, int fd, int)
 {
-	return readsubfonti(d, name, fd, nil, dolock);
+	return readsubfonti(d, name, fd, nil, 0);
 }
--- a/sys/src/libdraw/replclipr.c
+++ b/sys/src/libdraw/replclipr.c
@@ -7,8 +7,10 @@
 {
 	uchar *b;
 
+	_lockdisplay(i->display);
 	b = bufimage(i->display, 22);
 	if(b == nil){
+		_unlockdisplay(i->display);
 		fprint(2, "replclipr: %r\n");
 		return;
 	}
@@ -20,6 +22,7 @@
 	BPLONG(b+10, clipr.min.y);
 	BPLONG(b+14, clipr.max.x);
 	BPLONG(b+18, clipr.max.y);
+	_unlockdisplay(i->display);
 	i->repl = repl;
 	i->clipr = clipr;
 }
--- a/sys/src/libdraw/string.c
+++ b/sys/src/libdraw/string.c
@@ -111,8 +111,10 @@
 		m = 47+2*n;
 		if(bg)
 			m += 4+2*4;
+		_lockdisplay(dst->display);
 		b = bufimage(dst->display, m);
 		if(b == nil){
+			_unlockdisplay(dst->display);
 			fprint(2, "string: %r\n");
 			break;
 		}
@@ -142,6 +144,7 @@
 		ec = &cbuf[n];
 		for(c=cbuf; c<ec; c++, b+=2)
 			BPSHORT(b, *c);
+		_unlockdisplay(dst->display);
 		pt.x += wid;
 		bgp.x += wid;
 		agefont(f);
--- a/sys/src/libdraw/unloadimage.c
+++ b/sys/src/libdraw/unloadimage.c
@@ -36,8 +36,10 @@
 				dy = Dy(r);
 			n = bpl*dy;
 		}
+		_lockdisplay(d);
 		a = bufimage(d, 1+4+4*4);
 		if(a == nil){
+			_unlockdisplay(d);
 			werrstr("unloadimage: %r");
 			return -1;
 		}
@@ -47,10 +49,15 @@
 		BPLONG(a+9, r.min.y);
 		BPLONG(a+13, r.min.x+dx);
 		BPLONG(a+17, r.min.y+dy);
+		_unlockdisplay(d);
 		if(flushimage(d, 0) < 0)
 			return -1;
-		if(read(d->fd, data, n) < 0)
+		_lockdisplay(d);
+		if(read(d->fd, data, n) < 0){
+			_unlockdisplay(d);
 			return -1;
+		}
+		_unlockdisplay(d);
 		data += bpl*dy;
 		r.min.y += dy;
 	}
--- a/sys/src/libdraw/window.c
+++ b/sys/src/libdraw/window.c
@@ -2,8 +2,6 @@
 #include <libc.h>
 #include <draw.h>
 
-typedef struct Memimage Memimage;
-
 static int	screenid;
 
 Screen*
@@ -22,13 +20,18 @@
 	s = malloc(sizeof(Screen));
 	if(s == nil)
 		return nil;
+	_lockdisplay(d);
 	if(!screenid)
 		screenid = getpid();
+	_unlockdisplay(d);
 	for(try=0; try<25; try++){
-		/* loop until find a free id */
+		/* loop until we find a free id */
+		_lockdisplay(d);
 		a = bufimage(d, 1+4+4+4+1);
-		if(a == nil)
+		if(a == nil){
+			_unlockdisplay(d);
 			break;
+		}
 		id = ++screenid & 0xffff;	/* old devdraw bug */
 		a[0] = 'A';
 		BPLONG(a+1, id);
@@ -35,6 +38,7 @@
 		BPLONG(a+5, image->id);
 		BPLONG(a+9, fill->id);
 		a[13] = public;
+		_unlockdisplay(d);
 		if(flushimage(d, 0) != -1)
 			goto Found;
 	}
@@ -60,8 +64,10 @@
 	s = malloc(sizeof(Screen));
 	if(s == nil)
 		return nil;
+	_lockdisplay(d);
 	a = bufimage(d, 1+4+4);
 	if(a == nil){
+		_unlockdisplay(d);
 Error:
 		free(s);
 		return nil;
@@ -69,6 +75,7 @@
 	a[0] = 'S';
 	BPLONG(a+1, id);
 	BPLONG(a+5, chan);
+	_unlockdisplay(d);
 	if(flushimage(d, 0) < 0)
 		goto Error;
 
@@ -88,13 +95,16 @@
 	if(s == nil)
 		return 0;
 	d = s->display;
+	_lockdisplay(d);
 	a = bufimage(d, 1+4);
 	if(a == nil){
+		_unlockdisplay(d);
 		free(s);
 		return -1;
 	}
 	a[0] = 'F';
 	BPLONG(a+1, s->id);
+	_unlockdisplay(d);
 	free(s);
 	return 1;
 }
@@ -115,8 +125,10 @@
 	if(i == nil)
 		return nil;
 	i->screen = s;
-	i->next = s->display->windows;
-	s->display->windows = i;
+	_lockdisplay(d);
+	i->next = d->windows;
+	d->windows = i;
+	_unlockdisplay(d);
 	return i;
 }
 
@@ -152,14 +164,18 @@
 
 	if(n==0)
 		return;
+	_lockdisplay(d);
 	b = bufimage(d, 1+1+2+4*n);
-	if(b == nil)
+	if(b == nil){
+		_unlockdisplay(d);
 		return;
+	}
 	b[0] = 't';
 	b[1] = top;
 	BPSHORT(b+2, n);
 	for(i=0; i<n; i++)
 		BPLONG(b+4+4*i, w[i]->id);
+	_unlockdisplay(d);
 }
 
 void
@@ -194,9 +210,12 @@
 	uchar *b;
 	Point delta;
 
+	_lockdisplay(w->display);
 	b = bufimage(w->display, 1+4+2*4+2*4);
-	if(b == nil)
+	if(b == nil){
+		_unlockdisplay(w->display);
 		return 0;
+	}
 	b[0] = 'o';
 	BPLONG(b+1, w->id);
 	BPLONG(b+5, log.x);
@@ -203,6 +222,7 @@
 	BPLONG(b+9, log.y);
 	BPLONG(b+13, scr.x);
 	BPLONG(b+17, scr.y);
+	_unlockdisplay(w->display);
 	delta = subpt(log, w->r.min);
 	w->r = rectaddpt(w->r, delta);
 	w->clipr = rectaddpt(w->clipr, delta);
--- a/sys/src/libdraw/writeimage.c
+++ b/sys/src/libdraw/writeimage.c
@@ -13,7 +13,7 @@
 };
 
 int
-writeimage(int fd, Image *i, int dolock)
+writeimage(int fd, Image *i, int)
 {
 	uchar *outbuf, *outp, *eout;		/* encoded data, pointer, end */
 	uchar *loutp;				/* start of encoded line */
@@ -47,11 +47,7 @@
 			return -1;
 		data = malloc(bpl);
 		for(miny = r.min.y; miny != r.max.y; miny++){
-			if(dolock)
-				lockdisplay(i->display);
 			nb = unloadimage(i, Rect(r.min.x, miny, r.max.x, miny+1), data, bpl);
-			if(dolock)
-				unlockdisplay(i->display);
 			if(nb != bpl)
 				goto ErrOut0;
 			if(write(fd, data, nb) != nb)
@@ -74,12 +70,8 @@
 			dy = chunk/bpl;
 		if(dy <= 0)
 			dy = 1;
-		if(dolock)
-			lockdisplay(i->display);
 		nb = unloadimage(i, Rect(r.min.x, miny, r.max.x, miny+dy),
 			data+(miny-r.min.y)*bpl, dy*bpl);
-		if(dolock)
-			unlockdisplay(i->display);
 		if(nb != dy*bpl)
 			goto ErrOut0;
 	}
-- 
⑨