Nil pointer dereference at concurent logging

serazoli commented 4 years ago

When i use two logger instance concurrently then i get "invalid memory address or nil pointer dereference" Here a sample code to reproduce the error

package main
import (
    graylog "github.com/gemnasium/logrus-graylog-hook/v3"
    "github.com/sirupsen/logrus"
    "sync"
)
func main() {
    var wg sync.WaitGroup

    var gelfHook = graylog.NewGraylogHook("xx.xx.xx.xx:xxx", map[string]interface{}{})

    log1 := logrus.New()
    log2 := logrus.New()

    log1.AddHook(gelfHook)
    log2.AddHook(gelfHook)

    for i := 0; i < 100; i++ {
        wg.Add(1)
        go func() {
            defer wg.Done()
            log1.Info("test log info")
            log2.Info("test log info")
        }()
    }
    wg.Wait()
}

and the error message

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x4d19da]

goroutine 23 [running]:
compress/flate.(*Writer).Close(...)
        /usr/lib/go-1.13/src/compress/flate/deflate.go:729
compress/gzip.(*Writer).Close(0xc0000be210, 0xc0004801e0, 0x97)
        /usr/lib/go-1.13/src/compress/gzip/gzip.go:248 +0x6a
github.com/gemnasium/logrus-graylog-hook/v3.(*Writer).WriteMessage(0xc0000d0070, 0xc00041a100, 0xc000141ac0, 0x0)
        /xxxx/go/pkg/mod/github.com/gemnasium/logrus-graylog-hook/v3@v3.0.2/gelf_writer.go:246 +0x2e4
github.com/gemnasium/logrus-graylog-hook/v3.(*GraylogHook).sendEntry(0xc0000ba120, 0xc000119340, 0x0, 0x0, 0x0)
        /xxxx/go/pkg/mod/github.com/gemnasium/logrus-graylog-hook/v3@v3.0.2/graylog_hook.go:247 +0xa2b
github.com/gemnasium/logrus-graylog-hook/v3.(*GraylogHook).Fire(0xc0000ba120, 0xc0001192d0, 0x0, 0x0)
        /xxxx/go/pkg/mod/github.com/gemnasium/logrus-graylog-hook/v3@v3.0.2/graylog_hook.go:120 +0x294
github.com/sirupsen/logrus.LevelHooks.Fire(0xc00009c750, 0x4, 0xc0001192d0, 0xc0001192d0, 0x40d3a4)
        /xxxx/go/pkg/mod/github.com/sirupsen/logrus@v1.4.2/hooks.go:28 +0x91
github.com/sirupsen/logrus.(*Entry).fireHooks(0xc0001192d0)
        /xxxx/go/pkg/mod/github.com/sirupsen/logrus@v1.4.2/entry.go:246 +0xa3
github.com/sirupsen/logrus.Entry.log(0xc0000d0150, 0xc0001104e0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /xxxx/go/pkg/mod/github.com/sirupsen/logrus@v1.4.2/entry.go:224 +0xf4
github.com/sirupsen/logrus.(*Entry).Log(0xc000119260, 0xc000000004, 0xc000141f98, 0x1, 0x1)
        /xxxx/go/pkg/mod/github.com/sirupsen/logrus@v1.4.2/entry.go:268 +0xeb
github.com/sirupsen/logrus.(*Logger).Log(0xc0000d0150, 0x4, 0xc000141f98, 0x1, 0x1)
        /xxxx/go/pkg/mod/github.com/sirupsen/logrus@v1.4.2/logger.go:192 +0x7d
github.com/sirupsen/logrus.(*Logger).Info(...)
        /xxxx/go/pkg/mod/github.com/sirupsen/logrus@v1.4.2/logger.go:206
main.main.func1(0xc0000b4250, 0xc0000d00e0, 0xc0000d0150)
        /xxxx/main.go:25 +0x110
created by main.main
        /xxxx/main.go:22 +0x26e

I think that something will be override because if add this lines

w.mu.Lock()
defer w.mu.Unlock()

to gelf_writer.go line 198 then everything work as expected

gravis commented 4 years ago

Thanks for reporting this. I've been trying to reproduce this, without success so far. I need a test to make sure we don't have any regression, and even with your code I couldn't make the tests panic.

rvflash commented 3 years ago

Hi, thank you for your work. When do you plan to tag this code in a patch ?

gravis commented 3 years ago

Sorry @rvflash, looks like I forgot to push the tag to this repo. Done! Thanks for using this project.

gemnasium / logrus-graylog-hook

Nil pointer dereference at concurent logging #49