This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
libgo patch committed: send on a closed channel panics
- From: Ian Lance Taylor <iant at google dot com>
- To: gcc-patches at gcc dot gnu dot org, gofrontend-dev at googlegroups dot com
- Date: Wed, 23 Mar 2011 14:14:01 -0700
- Subject: libgo patch committed: send on a closed channel panics
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
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:
}