Patchwork libgo patch committed: send on a closed channel panics

login
register
mail settings
Submitter Ian Taylor
Date March 23, 2011, 9:14 p.m.
Message ID <mcrd3lhleue.fsf@google.com>
Download mbox | patch
Permalink /patch/88116/
State New
Headers show

Comments

Ian Taylor - March 23, 2011, 9:14 p.m.
The Go language spec has been changed to say that sending on a closed
channel panics.  This patch implements that.  Also, there is no longer
any limit on the number of times a program may receive a value from a
closed channel.  Also, calling close on a closed channel panics.
Bootstrapped and tested on x86_64-unknown-linux-gnu.  Committed to
mainline.

Ian

Patch

diff -r 1845bb5cbcca libgo/runtime/channel.h
--- a/libgo/runtime/channel.h	Mon Mar 21 15:00:19 2011 -0700
+++ b/libgo/runtime/channel.h	Wed Mar 23 13:36:20 2011 -0700
@@ -36,8 +36,6 @@ 
   pthread_cond_t cond;
   /* The size of elements on this channel.  */
   size_t element_size;
-  /* Number of operations on closed channel.  */
-  unsigned short closed_op_count;
   /* True if a goroutine is waiting to send on a synchronous
      channel.  */
   _Bool waiting_to_send;
@@ -84,22 +82,15 @@ 
    acquired while this mutex is held.  */
 extern pthread_mutex_t __go_select_data_mutex;
 
-/* Maximum permitted number of operations on a closed channel.  */
-#define MAX_CLOSED_OPERATIONS (0x100)
-
 extern struct __go_channel *__go_new_channel (size_t, size_t);
 
 extern _Bool __go_synch_with_select (struct __go_channel *, _Bool);
 
 extern void __go_broadcast_to_select (struct __go_channel *);
 
-extern _Bool __go_send_acquire (struct __go_channel *, _Bool);
+extern void __go_send_acquire (struct __go_channel *, _Bool);
 
-#define SEND_NONBLOCKING_ACQUIRE_SPACE 0
-#define SEND_NONBLOCKING_ACQUIRE_NOSPACE 1
-#define SEND_NONBLOCKING_ACQUIRE_CLOSED 2
-
-extern int __go_send_nonblocking_acquire (struct __go_channel *);
+extern _Bool __go_send_nonblocking_acquire (struct __go_channel *);
 
 extern void __go_send_release (struct __go_channel *);
 
diff -r 1845bb5cbcca libgo/runtime/go-close.c
--- a/libgo/runtime/go-close.c	Mon Mar 21 15:00:19 2011 -0700
+++ b/libgo/runtime/go-close.c	Wed Mar 23 13:36:20 2011 -0700
@@ -5,6 +5,7 @@ 
    license that can be found in the LICENSE file.  */
 
 #include "go-assert.h"
+#include "go-panic.h"
 #include "channel.h"
 
 /* Close a channel.  After a channel is closed, sends are no longer
@@ -24,6 +25,13 @@ 
       __go_assert (i == 0);
     }
 
+  if (channel->is_closed)
+    {
+      i = pthread_mutex_unlock (&channel->lock);
+      __go_assert (i == 0);
+      __go_panic_msg ("close of closed channel");
+    }
+
   channel->is_closed = 1;
 
   i = pthread_cond_broadcast (&channel->cond);
diff -r 1845bb5cbcca libgo/runtime/go-new-channel.c
--- a/libgo/runtime/go-new-channel.c	Mon Mar 21 15:00:19 2011 -0700
+++ b/libgo/runtime/go-new-channel.c	Wed Mar 23 13:36:20 2011 -0700
@@ -39,7 +39,6 @@ 
   i = pthread_cond_init (&ret->cond, NULL);
   __go_assert (i == 0);
   ret->element_size = element_size;
-  ret->closed_op_count = 0;
   ret->waiting_to_send = 0;
   ret->waiting_to_receive = 0;
   ret->selected_for_send = 0;
diff -r 1845bb5cbcca libgo/runtime/go-rec-nb-small.c
--- a/libgo/runtime/go-rec-nb-small.c	Mon Mar 21 15:00:19 2011 -0700
+++ b/libgo/runtime/go-rec-nb-small.c	Wed Mar 23 13:36:20 2011 -0700
@@ -32,16 +32,6 @@ 
 	  ? channel->next_store == 0
 	  : channel->next_fetch == channel->next_store))
     {
-      if (channel->saw_close)
-	{
-	  ++channel->closed_op_count;
-	  if (channel->closed_op_count >= MAX_CLOSED_OPERATIONS)
-	    {
-	      i = pthread_mutex_unlock (&channel->lock);
-	      __go_assert (i == 0);
-	      __go_panic_msg ("too many operations on closed channel");
-	    }
-	}
       channel->saw_close = 1;
       __go_unlock_and_notify_selects (channel);
       return RECEIVE_NONBLOCKING_ACQUIRE_CLOSED;
diff -r 1845bb5cbcca libgo/runtime/go-rec-small.c
--- a/libgo/runtime/go-rec-small.c	Mon Mar 21 15:00:19 2011 -0700
+++ b/libgo/runtime/go-rec-small.c	Wed Mar 23 13:36:20 2011 -0700
@@ -123,12 +123,6 @@ 
 	      ? channel->next_store == 0
 	      : channel->next_fetch == channel->next_store))
 	{
-	  if (channel->saw_close)
-	    {
-	      ++channel->closed_op_count;
-	      if (channel->closed_op_count >= MAX_CLOSED_OPERATIONS)
-		__go_panic_msg ("too many operations on closed channel");
-	    }
 	  channel->saw_close = 1;
 	  channel->selected_for_receive = 0;
 	  __go_unlock_and_notify_selects (channel);
diff -r 1845bb5cbcca libgo/runtime/go-send-big.c
--- a/libgo/runtime/go-send-big.c	Mon Mar 21 15:00:19 2011 -0700
+++ b/libgo/runtime/go-send-big.c	Wed Mar 23 13:36:20 2011 -0700
@@ -21,8 +21,7 @@ 
   alloc_size = ((channel->element_size + sizeof (uint64_t) - 1)
 		/ sizeof (uint64_t));
 
-  if (!__go_send_acquire (channel, for_select))
-    return;
+  __go_send_acquire (channel, for_select);
 
   offset = channel->next_store * alloc_size;
   __builtin_memcpy (&channel->data[offset], val, channel->element_size);
diff -r 1845bb5cbcca libgo/runtime/go-send-nb-big.c
--- a/libgo/runtime/go-send-nb-big.c	Mon Mar 21 15:00:19 2011 -0700
+++ b/libgo/runtime/go-send-nb-big.c	Wed Mar 23 13:36:20 2011 -0700
@@ -17,9 +17,8 @@ 
   alloc_size = ((channel->element_size + sizeof (uint64_t) - 1)
 		/ sizeof (uint64_t));
 
-  int data = __go_send_nonblocking_acquire (channel);
-  if (data != SEND_NONBLOCKING_ACQUIRE_SPACE)
-    return data == SEND_NONBLOCKING_ACQUIRE_CLOSED;
+  if (!__go_send_nonblocking_acquire (channel))
+    return 0;
 
   offset = channel->next_store * alloc_size;
   __builtin_memcpy (&channel->data[offset], val, channel->element_size);
diff -r 1845bb5cbcca libgo/runtime/go-send-nb-small.c
--- a/libgo/runtime/go-send-nb-small.c	Mon Mar 21 15:00:19 2011 -0700
+++ b/libgo/runtime/go-send-nb-small.c	Wed Mar 23 13:36:20 2011 -0700
@@ -10,9 +10,11 @@ 
 #include "go-panic.h"
 #include "channel.h"
 
-/* Prepare to send something on a nonblocking channel.  */
+/* Prepare to send something on a nonblocking channel.  Return true if
+   we acquired the channel, false if we did not acquire it because
+   there is no space to send a value.  */
 
-int
+_Bool
 __go_send_nonblocking_acquire (struct __go_channel *channel)
 {
   int i;
@@ -29,16 +31,9 @@ 
 
   if (channel->is_closed)
     {
-      ++channel->closed_op_count;
-      if (channel->closed_op_count >= MAX_CLOSED_OPERATIONS)
-	{
-	  i = pthread_mutex_unlock (&channel->lock);
-	  __go_assert (i == 0);
-	  __go_panic_msg ("too many operations on closed channel");
-	}
       i = pthread_mutex_unlock (&channel->lock);
       __go_assert (i == 0);
-      return SEND_NONBLOCKING_ACQUIRE_CLOSED;
+      __go_panic_msg ("send on closed channel");
     }
 
   if (channel->num_entries > 0)
@@ -87,10 +82,10 @@ 
       i = pthread_mutex_unlock (&channel->lock);
       __go_assert (i == 0);
 
-      return SEND_NONBLOCKING_ACQUIRE_NOSPACE;
+      return 0;
     }
 
-  return SEND_NONBLOCKING_ACQUIRE_SPACE;
+  return 1;
 }
 
 /* Send something 64 bits or smaller on a channel.  */
@@ -100,9 +95,8 @@ 
 {
   __go_assert (channel->element_size <= sizeof (uint64_t));
 
-  int data = __go_send_nonblocking_acquire (channel);
-  if (data != SEND_NONBLOCKING_ACQUIRE_SPACE)
-    return data == SEND_NONBLOCKING_ACQUIRE_CLOSED;
+  if (!__go_send_nonblocking_acquire (channel))
+    return 0;
 
   channel->data[channel->next_store] = val;
 
diff -r 1845bb5cbcca libgo/runtime/go-send-small.c
--- a/libgo/runtime/go-send-small.c	Mon Mar 21 15:00:19 2011 -0700
+++ b/libgo/runtime/go-send-small.c	Wed Mar 23 13:36:20 2011 -0700
@@ -10,12 +10,11 @@ 
 #include "go-panic.h"
 #include "channel.h"
 
-/* Prepare to send something on a channel.  Return true if the channel
-   is acquired, false, if it is closed.  FOR_SELECT is true if this
+/* Prepare to send something on a channel.  FOR_SELECT is true if this
    call is being made after a select statement returned with this
    channel selected.  */
 
-_Bool
+void
 __go_send_acquire (struct __go_channel *channel, _Bool for_select)
 {
   int i;
@@ -25,19 +24,13 @@ 
 
   while (1)
     {
-      /* Check whether the channel is closed.  */
       if (channel->is_closed)
 	{
-	  ++channel->closed_op_count;
-	  if (channel->closed_op_count >= MAX_CLOSED_OPERATIONS)
-	    {
-	      i = pthread_mutex_unlock (&channel->lock);
-	      __go_assert (i == 0);
-	      __go_panic_msg ("too many operations on closed channel");
-	    }
-	  channel->selected_for_send = 0;
-	  __go_unlock_and_notify_selects (channel);
-	  return 0;
+	  if (for_select)
+	    channel->selected_for_send = 0;
+	  i = pthread_mutex_unlock (&channel->lock);
+	  __go_assert (i == 0);
+	  __go_panic_msg ("send on closed channel");
 	}
 
       /* If somebody else has the channel locked for sending, we have
@@ -54,7 +47,7 @@ 
 	      if (!channel->waiting_to_send)
 		{
 		  __go_assert (channel->next_store == 0);
-		  return 1;
+		  return;
 		}
 	    }
 	  else
@@ -62,7 +55,7 @@ 
 	      /* If there is room on the channel, we are OK.  */
 	      if ((channel->next_store + 1) % channel->num_entries
 		  != channel->next_fetch)
-		return 1;
+		return;
 	    }
 	}
 
@@ -156,8 +149,7 @@ 
 
   __go_assert (channel->element_size <= sizeof (uint64_t));
 
-  if (!__go_send_acquire (channel, for_select))
-    return;
+  __go_send_acquire (channel, for_select);
 
   channel->data[channel->next_store] = val;
 
Index: closedchan.go
===================================================================
--- closedchan.go	(revision 170881)
+++ closedchan.go	(working copy)
@@ -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())
 	}
Index: chan/select3.go
===================================================================
--- chan/select3.go	(revision 170881)
+++ chan/select3.go	(working copy)
@@ -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:
 		}