From: Russ Cox <rsc@golang.org>
Date: Fri, 21 Jan 2011 20:07:13 +0000 (-0500)
Subject: spec, runtime, tests: send on closed channel panics
X-Git-Tag: weekly.2011-02-01~107
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=27c74d3499b12288fb4a944ce5376820dee1c8b1;p=gostls13.git

spec, runtime, tests: send on closed channel panics

Close of closed channel panics.
Receive from closed channel never panics,
even if done repeatedly.

Fixes #1349.
Fixes #1419.

R=gri, iant, ken2, r, gri1, r2, iant2, rog, albert.strasheim, niemeyer, ejsherry
CC=golang-dev
https://golang.org/cl/3989042
---

diff --git a/doc/go_spec.html b/doc/go_spec.html
index f2e55a02c4..f3ffceb946 100644
--- a/doc/go_spec.html
+++ b/doc/go_spec.html
@@ -4394,7 +4394,7 @@ BuiltinArgs = Type [ "," ExpressionList ] | ExpressionList .
 <p>
 For a channel <code>c</code>, the built-in function <code>close(c)</code>
 marks the channel as unable to accept more values through a send operation;
-values sent to a closed channel are ignored.
+sending to or closing a closed channel causes a <a href="#Run_time_panics">run-time panic</a>.
 After calling <code>close</code>, and after any previously
 sent values have been received, receive operations will return
 the zero value for the channel's type without blocking.
diff --git a/src/pkg/runtime/chan.c b/src/pkg/runtime/chan.c
index 4ef0b6f6f5..1cc589278d 100644
--- a/src/pkg/runtime/chan.c
+++ b/src/pkg/runtime/chan.c
@@ -11,8 +11,6 @@ enum
 {
 	Wclosed		= 0x0001,	// writer has closed
 	Rclosed		= 0x0002,	// reader has seen close
-	Eincr		= 0x0004,	// increment errors
-	Emax		= 0x0800,	// error limit before throw
 };
 
 typedef	struct	Link	Link;
@@ -151,16 +149,6 @@ runtime·makechan(Type *elem, int64 hint, Hchan *ret)
 	FLUSH(&ret);
 }
 
-static void
-incerr(Hchan* c)
-{
-	c->closed += Eincr;
-	if(c->closed & Emax) {
-		// Note that channel locks may still be held at this point.
-		runtime·throw("too many operations on a closed channel");
-	}
-}
-
 /*
  * generic single channel send/recv
  * if the bool pointer is nil,
@@ -276,10 +264,8 @@ asynch:
 	return;
 
 closed:
-	incerr(c);
-	if(pres != nil)
-		*pres = true;
 	runtime·unlock(c);
+	runtime·panicstring("send on closed channel");
 }
 
 void
@@ -393,7 +379,6 @@ closed:
 		*closed = true;
 	c->elemalg->copy(c->elemsize, ep, nil);
 	c->closed |= Rclosed;
-	incerr(c);
 	if(pres != nil)
 		*pres = true;
 	runtime·unlock(c);
@@ -863,7 +848,6 @@ rclose:
 	if(cas->u.elemp != nil)
 		c->elemalg->copy(c->elemsize, cas->u.elemp, nil);
 	c->closed |= Rclosed;
-	incerr(c);
 	goto retc;
 
 syncsend:
@@ -876,12 +860,6 @@ syncsend:
 	gp = sg->g;
 	gp->param = sg;
 	runtime·ready(gp);
-	goto retc;
-
-sclose:
-	// send on closed channel
-	incerr(c);
-	goto retc;
 
 retc:
 	selunlock(sel);
@@ -891,6 +869,12 @@ retc:
 	as = (byte*)&sel + cas->so;
 	freesel(sel);
 	*as = true;
+	return;
+
+sclose:
+	// send on closed channel
+	selunlock(sel);
+	runtime·panicstring("send on closed channel");
 }
 
 // closechan(sel *byte);
@@ -904,7 +888,11 @@ runtime·closechan(Hchan *c)
 		runtime·gosched();
 
 	runtime·lock(c);
-	incerr(c);
+	if(c->closed & Wclosed) {
+		runtime·unlock(c);
+		runtime·panicstring("close of closed channel");
+	}
+
 	c->closed |= Wclosed;
 
 	// release all readers
diff --git a/test/chan/select3.go b/test/chan/select3.go
index a1a2ef50b5..9877b12a98 100644
--- a/test/chan/select3.go
+++ b/test/chan/select3.go
@@ -97,13 +97,9 @@ func main() {
 		}
 	})
 
-	// sending (a small number of times) to a closed channel is not specified
-	// but the current implementation doesn't block: test that different
-	// implementations behave the same
-	testBlock(never, func() {
-		for i := 0; i < 10; i++ {
-			closedch <- 7
-		}
+	// sending to a closed channel panics.
+	testPanic(always, func() {
+		closedch <- 7
 	})
 
 	// receiving from a non-ready channel always blocks
@@ -189,13 +185,13 @@ func main() {
 		}
 	})
 
-	// selects with closed channels don't block
+	// selects with closed channels behave like ordinary operations
 	testBlock(never, func() {
 		select {
 		case <-closedch:
 		}
 	})
-	testBlock(never, func() {
+	testPanic(always, func() {
 		select {
 		case closedch <- 7:
 		}
diff --git a/test/closedchan.go b/test/closedchan.go
index c7c759be3b..8126d5a4e4 100644
--- a/test/closedchan.go
+++ b/test/closedchan.go
@@ -100,6 +100,15 @@ func (c SChan) Impl() string {
 	return "(select)"
 }
 
+func shouldPanic(f func()) {
+	defer func() {
+		if recover() == nil {
+			panic("did not panic")
+		}
+	}()
+	f()
+}
+
 func test1(c Chan) {
 	// not closed until the close signal (a zero value) has been received.
 	if c.Closed() {
@@ -128,18 +137,15 @@ func test1(c Chan) {
 	}
 
 	// send should work with ,ok too: sent a value without blocking, so ok == true.
-	ok := c.Nbsend(1)
-	if !ok {
-		println("test1: send on closed got not ok", c.Impl())
-	}
+	shouldPanic(func(){c.Nbsend(1)})
 
-	// but the value should have been discarded.
+	// the value should have been discarded.
 	if x := c.Recv(); x != 0 {
 		println("test1: recv on closed got non-zero after send on closed:", x, c.Impl())
 	}
 
 	// similarly Send.
-	c.Send(2)
+	shouldPanic(func(){c.Send(2)})
 	if x := c.Recv(); x != 0 {
 		println("test1: recv on closed got non-zero after send on closed:", x, c.Impl())
 	}