Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

log: GC crash using logger in 1.4 #9425

Closed
vinibaggio opened this issue Dec 22, 2014 · 5 comments
Closed

log: GC crash using logger in 1.4 #9425

vinibaggio opened this issue Dec 22, 2014 · 5 comments
Assignees
Milestone

Comments

@vinibaggio
Copy link

@vinibaggio vinibaggio commented Dec 22, 2014

I am having a garbage collection crash (dump) in 1.4 in the code snipped below.
In 1.3.3, the same program prints "Hello" and successfully exits:

package main

import (
    "log"
    "reflect"
    "sync"
)

type Bus struct {
    listeners map[reflect.Type][]reflect.Value
}

// Register adds a new event listener.
// Assumes a function with one parameter as fn argument
func (b *Bus) Register(fn interface{}) {
    v := reflect.ValueOf(fn)
    t := v.Type()

    // Map the argument type to the listener.
    arg := t.In(0)
    b.listeners[arg] = append(b.listeners[arg], v)
}

// Emit dispatches an event to all registered listeners.
// A WaitGroup is returned which can optionally be used to block on all handlers
// completing (note: the bus makes no attempt to ensure the handlers actually
// complete).
func (b *Bus) Emit(event interface{}) *sync.WaitGroup {
    var v reflect.Value
    args := make([]reflect.Value, 1)

    v = reflect.ValueOf(event)
    args[0] = v

    var wg sync.WaitGroup
    for _, fn := range b.listeners[v.Type()] {
        wg.Add(1)
        go func() {
            fn.Call(args)
            wg.Done()
        }()
    }
    return &wg
}

func main() {
    bus := &Bus{
        listeners: map[reflect.Type][]reflect.Value{},
    }

    bus.Register(func(i int) {
        log.Println("Hello")
    })

    wg := bus.Emit(1)
    wg.Wait()
}

My go version is 1.4 in OSX, using Homebrew package management:

$ go version
go version go1.4 darwin/amd64

For reference: golang-nuts thread on the issue

@vinibaggio vinibaggio changed the title GC crash using logger in 1.4 log: GC crash using logger in 1.4 Dec 22, 2014
@josharian
Copy link
Contributor

@josharian josharian commented Dec 23, 2014

Here's a smaller reproduction:

package main

import (
    "log"
    "reflect"
)

func main() {
    f := func(i int) {
        log.Println("Hello")
    }
    fn := reflect.ValueOf(f)
    args := []reflect.Value{reflect.ValueOf(1)}
    fn.Call(args)
}

cc @randall77

@josharian josharian added accepted and removed accepted labels Dec 23, 2014
@randall77
Copy link
Contributor

@randall77 randall77 commented Dec 23, 2014

This feels similar to #9179

@randall77 randall77 self-assigned this Dec 23, 2014
@randall77
Copy link
Contributor

@randall77 randall77 commented Dec 23, 2014

The bug is in malloc and/or funcLayout. Malloc assumes 8-byte objects (which is what the argument frame is) either have to be marked as flagNoScan or contain a pointer:

    if size == ptrSize {
        // It's one word and it has pointers, it must be a pointer.
        *xbits |= (bitsPointer << 2) << shift
        goto marked
    }

but reflect doesn't set flagNoScan, even though the GC program it generates is correct. I'll fix it.

@randall77 randall77 added this to the Go1.4.1 milestone Dec 23, 2014
@vinibaggio
Copy link
Author

@vinibaggio vinibaggio commented Dec 23, 2014

This is awesome, thank you for the super fast response and catching the bug!

@randall77
Copy link
Contributor

@randall77 randall77 commented Dec 23, 2014

@gopherbot gopherbot closed this in d11f411 Dec 23, 2014
gopherbot pushed a commit that referenced this issue Dec 23, 2014
Test more stuff:
1) flagNoPointers, an incorrect value was the cause of #9425
2) Total function layout size
3) gc program

Change-Id: I73f65fe740215938fa930d2f096febd9db0a0021
Reviewed-on: https://go-review.googlesource.com/2090
Reviewed-by: Ian Lance Taylor <iant@golang.org>
rsc added a commit that referenced this issue Jan 14, 2015
…ut has no pointers.

malloc checks kindNoPointers and if it is not set and the object
is one pointer in size, it assumes it contains a pointer.  So we
must set kindNoPointers correctly; it isn't just a hint.

Fixes #9425

Change-Id: Ia43da23cc3298d6e3d6dbdf66d32e9678f0aedcf
Reviewed-on: https://go-review.googlesource.com/2055
Reviewed-by: Russ Cox <rsc@golang.org>
(cherry picked from commit d11f411)
Reviewed-on: https://go-review.googlesource.com/2800
Reviewed-by: Andrew Gerrand <adg@golang.org>
@golang golang locked and limited conversation to collaborators Jun 25, 2016
wheatman added a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
…ut has no pointers.

malloc checks kindNoPointers and if it is not set and the object
is one pointer in size, it assumes it contains a pointer.  So we
must set kindNoPointers correctly; it isn't just a hint.

Fixes golang#9425

Change-Id: Ia43da23cc3298d6e3d6dbdf66d32e9678f0aedcf
Reviewed-on: https://go-review.googlesource.com/2055
Reviewed-by: Russ Cox <rsc@golang.org>
(cherry picked from commit d11f411)
Reviewed-on: https://go-review.googlesource.com/2800
Reviewed-by: Andrew Gerrand <adg@golang.org>
wheatman added a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
…ut has no pointers.

malloc checks kindNoPointers and if it is not set and the object
is one pointer in size, it assumes it contains a pointer.  So we
must set kindNoPointers correctly; it isn't just a hint.

Fixes golang#9425

Change-Id: Ia43da23cc3298d6e3d6dbdf66d32e9678f0aedcf
Reviewed-on: https://go-review.googlesource.com/2055
Reviewed-by: Russ Cox <rsc@golang.org>
(cherry picked from commit d11f411)
Reviewed-on: https://go-review.googlesource.com/2800
Reviewed-by: Andrew Gerrand <adg@golang.org>
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
…ut has no pointers.

malloc checks kindNoPointers and if it is not set and the object
is one pointer in size, it assumes it contains a pointer.  So we
must set kindNoPointers correctly; it isn't just a hint.

Fixes golang#9425

Change-Id: Ia43da23cc3298d6e3d6dbdf66d32e9678f0aedcf
Reviewed-on: https://go-review.googlesource.com/2055
Reviewed-by: Russ Cox <rsc@golang.org>
(cherry picked from commit d11f411)
Reviewed-on: https://go-review.googlesource.com/2800
Reviewed-by: Andrew Gerrand <adg@golang.org>
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 20, 2018
…ut has no pointers.

malloc checks kindNoPointers and if it is not set and the object
is one pointer in size, it assumes it contains a pointer.  So we
must set kindNoPointers correctly; it isn't just a hint.

Fixes golang#9425

Change-Id: Ia43da23cc3298d6e3d6dbdf66d32e9678f0aedcf
Reviewed-on: https://go-review.googlesource.com/2055
Reviewed-by: Russ Cox <rsc@golang.org>
(cherry picked from commit d11f411)
Reviewed-on: https://go-review.googlesource.com/2800
Reviewed-by: Andrew Gerrand <adg@golang.org>
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
…ut has no pointers.

malloc checks kindNoPointers and if it is not set and the object
is one pointer in size, it assumes it contains a pointer.  So we
must set kindNoPointers correctly; it isn't just a hint.

Fixes golang#9425

Change-Id: Ia43da23cc3298d6e3d6dbdf66d32e9678f0aedcf
Reviewed-on: https://go-review.googlesource.com/2055
Reviewed-by: Russ Cox <rsc@golang.org>
(cherry picked from commit d11f411)
Reviewed-on: https://go-review.googlesource.com/2800
Reviewed-by: Andrew Gerrand <adg@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.