Grokbase › Groups › Go › golang-dev › December 2012
FAQ
Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://dvyukov%40google.com@code.google.com/p/go/


Description:
runtime: diagnose double wakeup on Note
Double wakeup is prohibited by the Note interface
and checked in lock_sema.c.

Please review this at https://codereview.appspot.com/6976054/

Affected files:
M src/pkg/runtime/lock_futex.c


Index: src/pkg/runtime/lock_futex.c
===================================================================
--- a/src/pkg/runtime/lock_futex.c
+++ b/src/pkg/runtime/lock_futex.c
@@ -111,7 +111,8 @@
void
runtime·notewakeup(Note *n)
{
- runtime·xchg(&n->key, 1);
+ if(runtime·xchg(&n->key, 1))
+ runtime·throw("notewakeup - double wakeup");
runtime·futexwakeup(&n->key, 1);
}

Search Discussions

  • Brad Fitzpatrick at Dec 24, 2012 at 4:57 pm ⇧
    LGTM
    On Mon, Dec 24, 2012 at 8:38 AM, wrote:

    Reviewers: golang-dev_googlegroups.com,

    Message:
    Hello golang-dev@googlegroups.com,

    I'd like you to review this change to
    https://dvyukov%40google.com@**code.google.com/p/go/<http://40google.com@code.google.com/p/go/>


    Description:
    runtime: diagnose double wakeup on Note
    Double wakeup is prohibited by the Note interface
    and checked in lock_sema.c.

    Please review this at https://codereview.appspot.**com/6976054/<https://codereview.appspot.com/6976054/>

    Affected files:
    M src/pkg/runtime/lock_futex.c


    Index: src/pkg/runtime/lock_futex.c
    ==============================**==============================**=======
    --- a/src/pkg/runtime/lock_futex.c
    +++ b/src/pkg/runtime/lock_futex.c
    @@ -111,7 +111,8 @@
    void
    runtime·notewakeup(Note *n)
    {
    - runtime·xchg(&n->key, 1);
    + if(runtime·xchg(&n->key, 1))
    + runtime·throw("notewakeup - double wakeup");
    runtime·futexwakeup(&n->key, 1);
    }


  • Dvyukov at Dec 24, 2012 at 5:07 pm ⇧
    *** Submitted as
    https://code.google.com/p/go/source/detail?r=aa5d9f234a8e ***

    runtime: diagnose double wakeup on Note
    Double wakeup is prohibited by the Note interface
    and checked in lock_sema.c.

    R=golang-dev, bradfitz
    CC=golang-dev
    https://codereview.appspot.com/6976054


    https://codereview.appspot.com/6976054/
  • Brad Fitzpatrick at Jan 8, 2013 at 8:16 pm ⇧
    Dmitri,

    I just got this crash from this CL at 74e2affcfe39.

    It went away after my next run of "go test -short std", though. I only saw
    it once:

    fatal error: notewakeup - double wakeup

    goroutine 0 [idle]:

    goroutine 1 [chan receive]:
    testing.RunTests(0x400c00, 0x5d25d0, 0x2, 0x2, 0x1, ...)
    /home/bradfitz/go/src/pkg/testing/testing.go:378 +0x891
    testing.Main(0x400c00, 0x5d25d0, 0x2, 0x2, 0x5d8c48, ...)
    /home/bradfitz/go/src/pkg/testing/testing.go:313 +0x8a
    main.main()
    os/signal/_test/_testmain.go:45 +0x9a

    goroutine 2 [syscall]:
    created by runtime.main
    /home/bradfitz/go/src/pkg/runtime/proc.c:225

    goroutine 3 [syscall]:
    os/signal.loop()
    /home/bradfitz/go/src/pkg/os/signal/signal_unix.go:20 +0x1c
    created by os/signal.init·1
    /home/bradfitz/go/src/pkg/os/signal/signal_unix.go:26 +0x2f

    goroutine 6 [sleep]:
    time.Sleep(0x5f5e100, 0xc20005d140)
    /home/bradfitz/go/src/pkg/runtime/ztime_linux_amd64.c:20 +0x2f
    os/signal.TestStress()
    /home/bradfitz/go/src/pkg/os/signal/signal_test.go:97 +0x141
    testing.tRunner(0xc200085090, 0x5d25e8, 0x0, 0x0)
    /home/bradfitz/go/src/pkg/testing/testing.go:301 +0x6c
    created by testing.RunTests
    /home/bradfitz/go/src/pkg/testing/testing.go:377 +0x86e

    goroutine 5 [syscall]:
    created by addtimer
    /home/bradfitz/go/src/pkg/runtime/ztime_linux_amd64.c:73

    goroutine 7 [select]:
    os/signal.func·001(0xc20005d138, 0xc20005d140, 0x0, 0x0)
    /home/bradfitz/go/src/pkg/os/signal/signal_test.go:76 +0x14d
    created by os/signal.TestStress
    /home/bradfitz/go/src/pkg/os/signal/signal_test.go:83 +0x110

    goroutine 8 [running]:
    created by os/signal.TestStress
    /home/bradfitz/go/src/pkg/os/signal/signal_test.go:96 +0x131
    FAIL os/signal 0.094s

    On Mon, Dec 24, 2012 at 8:38 AM, wrote:

    Reviewers: golang-dev_googlegroups.com,

    Message:
    Hello golang-dev@googlegroups.com,

    I'd like you to review this change to
    https://dvyukov%40google.com@**code.google.com/p/go/<http://40google.com@code.google.com/p/go/>


    Description:
    runtime: diagnose double wakeup on Note
    Double wakeup is prohibited by the Note interface
    and checked in lock_sema.c.

    Please review this at https://codereview.appspot.**com/6976054/<https://codereview.appspot.com/6976054/>

    Affected files:
    M src/pkg/runtime/lock_futex.c


    Index: src/pkg/runtime/lock_futex.c
    ==============================**==============================**=======
    --- a/src/pkg/runtime/lock_futex.c
    +++ b/src/pkg/runtime/lock_futex.c
    @@ -111,7 +111,8 @@
    void
    runtime·notewakeup(Note *n)
    {
    - runtime·xchg(&n->key, 1);
    + if(runtime·xchg(&n->key, 1))
    + runtime·throw("notewakeup - double wakeup");
    runtime·futexwakeup(&n->key, 1);
    }


  • Dmitry Vyukov at Jan 9, 2013 at 7:08 am ⇧

    On Wed, Jan 9, 2013 at 12:16 AM, Brad Fitzpatrick wrote:
    Dmitri,

    I just got this crash from this CL at 74e2affcfe39.

    It went away after my next run of "go test -short std", though. I only saw
    it once:

    fatal error: notewakeup - double wakeup

    Is it linux?
    I guess you don't have local changes in runtime, right?
    I've run os/signal test several hundreds times and can't reproduce it...
    Can you try to reproduce it with GOTRACEBACK=2 (it's useful to have in bashrc)?
  • Dmitry Vyukov at Jan 9, 2013 at 7:22 am ⇧

    On Wed, Jan 9, 2013 at 11:00 AM, Dmitry Vyukov wrote:
    On Wed, Jan 9, 2013 at 12:16 AM, Brad Fitzpatrick wrote:
    Dmitri,

    I just got this crash from this CL at 74e2affcfe39.

    It went away after my next run of "go test -short std", though. I only saw
    it once:

    fatal error: notewakeup - double wakeup

    Is it linux?
    I guess you don't have local changes in runtime, right?
    I've run os/signal test several hundreds times and can't reproduce it...
    Can you try to reproduce it with GOTRACEBACK=2 (it's useful to have in bashrc)?
    What about adding GOTRACEBACK=2 to run.bash?
    https://codereview.appspot.com/7069060
    If it even happen on bots, it can provide some additional information.
  • Brad Fitzpatrick at Jan 9, 2013 at 10:58 pm ⇧

    On Tue, Jan 8, 2013 at 11:00 PM, Dmitry Vyukov wrote:
    On Wed, Jan 9, 2013 at 12:16 AM, Brad Fitzpatrick wrote:
    Dmitri,

    I just got this crash from this CL at 74e2affcfe39.

    It went away after my next run of "go test -short std", though. I only saw
    it once:

    fatal error: notewakeup - double wakeup

    Is it linux?
    yes

    I guess you don't have local changes in runtime, right?
    no runtime changes
  • Dmitry Vyukov at Jan 10, 2013 at 5:55 am ⇧

    On Thu, Jan 10, 2013 at 2:58 AM, Brad Fitzpatrick wrote:
    wrote:
    Dmitri,

    I just got this crash from this CL at 74e2affcfe39.

    It went away after my next run of "go test -short std", though. I only
    saw
    it once:

    fatal error: notewakeup - double wakeup

    Is it linux?

    yes
    I guess you don't have local changes in runtime, right?

    no runtime changes

    I will inspect usages of Note, but without knowing what Note it is,
    it's unlikely to succeed...

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedDec 24, '12 at 4:38p
activeJan 10, '13 at 5:55a
posts8
users2
websitegolang.org

2 users in discussion

Dmitry Vyukov: 5 posts Brad Fitzpatrick: 3 posts

Content

People

Support

Translate

site design / logo © 2022 Grokbase