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 f2e55a02c4066929354f44652749bccf7cfc6dbf..f3ffceb946c9049d5bfc7a5ccc0217936bf46a45 100644 (file)
--- 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 4ef0b6f6f509bcb97d4e520b9fd51e51b44e2e2e..1cc589278d9ce706f4845ffec893839cbfe2d8c3 100644 (file)
--- 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 a1a2ef50b56ed38ca935e3bf013406728779c41b..9877b12a98ce04e1f481453d0809f356fa8afae4 100644 (file)
--- 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 c7c759be3b3edfcad980be8dd0155493db7f7aab..8126d5a4e4c71ade746f8e836d7f25451cf19d73 100644 (file)
--- 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())
        }