RichardKnop / machinery

Machinery is an asynchronous task queue/job queue based on distributed message passing.
Mozilla Public License 2.0
7.57k stars 917 forks source link

Chord task with redis backend raise nil pointer panic #619

Open archerbj opened 4 years ago

archerbj commented 4 years ago

I am using RabbitMQ as a broker and Redis as the backend to run the example code.

Here is the server setup code

backend := redisbackend.NewGR(cnf, []string{"localhost:6379"}, 0)
    cnf.ResultBackend = "redis://localhost:6379"
    cnf.Redis = &config.RedisConfig{
        MaxIdle:                4,
        IdleTimeout:            240,
        ReadTimeout:            15,
        WriteTimeout:           15,
        ConnectTimeout:         15,
        NormalTasksPollPeriod:  1000,
        DelayedTasksPollPeriod: 500,
    }

When the code runs to send a chord task, the worker raised this error

INFO: 2020/11/16 16:40:41 machinery.go:163  I am an end of task handler for: add 
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x1a248d6]

goroutine 32 [running]:
github.com/RichardKnop/redsync.(*Redsync).NewMutex(0x0, 0x1ccbb1b, 0x11, 0x0, 0x0, 0x0, 0xc0002606e0)
        /Users/user/coding/go/eti/pkg/mod/github.com/!richard!knop/redsync@v1.2.0/redsync.go:25 +0x26
github.com/RichardKnop/machinery/v1/backends/redis.(*BackendGR).TriggerChord(0xc0000fe3f0, 0xc0005b9860, 0x2a, 0x0, 0x0, 0x0)
        /Users/user/coding/go/eti/src/machinery/machinery/v1/backends/redis/goredis.go:118 +0x7a
github.com/RichardKnop/machinery/v2.(*Worker).taskSucceeded(0xc000100230, 0xc0004a8000, 0xc000586020, 0x1, 0x1, 0x0, 0x0)
        /Users/user/coding/go/eti/src/machinery/machinery/v2/worker.go:309 +0x5d2
github.com/RichardKnop/machinery/v2.(*Worker).Process(0xc000100230, 0xc0004a8000, 0x0, 0x0)
        /Users/user/coding/go/eti/src/machinery/machinery/v2/worker.go:200 +0x599
github.com/RichardKnop/machinery/v1/brokers/amqp.(*Broker).consumeOne(0xc0000ea7e0, 0x200ac80, 0xc0000ec7e0, 0x0, 0xc000577500, 0x10, 0x0, 0x0, 0x2, 0x0, ...)
        /Users/user/coding/go/eti/src/machinery/machinery/v1/brokers/amqp/amqp.go:336 +0x4f4
github.com/RichardKnop/machinery/v1/brokers/amqp.(*Broker).consume.func2(0xc0000ea7e0, 0xc000220640, 0x200aa80, 0xc000100230, 0xc0000a24e0, 0x0, 0xc0001a8180)
        /Users/user/coding/go/eti/src/machinery/machinery/v1/brokers/amqp/amqp.go:286 +0xca
created by github.com/RichardKnop/machinery/v1/brokers/amqp.(*Broker).consume
        /Users/user/coding/go/eti/src/machinery/machinery/v1/brokers/amqp/amqp.go:285 +0x133
exit status 2

It seems the NewGR method forgot to instantiate redsync?

// NewGR creates Backend instance
func NewGR(cnf *config.Config, addrs []string, db int) iface.Backend {
    b := &BackendGR{
        Backend: common.NewBackend(cnf),
    }
    parts := strings.Split(addrs[0], "@")
    if len(parts) == 2 {
        // with passwrod
        b.password = parts[0]
        addrs[0] = parts[1]
    }

    ropt := &redis.UniversalOptions{
        Addrs:    addrs,
        DB:       db,
        Password: b.password,
    }
    if cnf.Redis != nil {
        ropt.MasterName = cnf.Redis.MasterName
    }
    b.rclient = redis.NewUniversalClient(ropt)
    return b
}
RichardKnop commented 4 years ago

@archerbj Could you submit a PR with a fix?

vincenthcui commented 4 years ago

@archerbj I have submitted PR #620 which solve the problem and passed tests, I hope it will work for you :) @RichardKnop would you please help review the pull request, and evaluate the side effect