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

Nil context field on message #177

Closed
elffjs opened this issue May 19, 2022 · 0 comments
Closed

Nil context field on message #177

elffjs opened this issue May 19, 2022 · 0 comments

Comments

@elffjs
Copy link

elffjs commented May 19, 2022

While testing out taskq together with Redis, we found that our applications would panic, usually after a task failure, on a nil pointer dereference here in go-redis:

func (p *ConnPool) waitTurn(ctx context.Context) error {
	select {
	case <-ctx.Done():

Evidently, ctx was nil. The traces were somewhat obscured in cluster mode because of the additional goroutines, but in non-cluster mode we saw this section:

github.com/go-redis/redis/v8.(*Pipeline).Exec(0xc0008eb4d0?, {0x0?, 0x0?})
    /go/src/…/vendor/github.com/go-redis/redis/v8/pipeline.go:125 +0x15f
github.com/vmihailenco/taskq/v3/redisq.(*Queue).Release(0xc0008eb4d0, 0xc000660d00)
    /go/src/…/vendor/github.com/vmihailenco/taskq/v3/redisq/queue.go:245 +0x1ae
github.com/vmihailenco/taskq/v3.(*Consumer).fetchMessages(0xc00002c270, {0x1f41030, 0xc000044090}, 0xc0006b26e0, 0x2?)
    /go/src/…/vendor/github.com/vmihailenco/taskq/v3/consumer.go:439 +0x29f

It seems that msg.Ctx is nil here:

_, err = pipe.Exec(msg.Ctx)

Indeed, msg is not touched much after msgpack creates it. Looking at a similar part of queue.go we see an explicit initialization of the Ctx field:

taskq/redisq/queue.go

Lines 408 to 410 in 8faa953

msg := new(taskq.Message)
msg.Ctx = ctx
err = unmarshalMessage(msg, xmsg)

Which was added in response to a nil pointer in #150. Adding a similar initialization seems to work here as well.

kd7lxl added a commit to kd7lxl/taskq that referenced this issue Oct 12, 2022
lilien1010 added a commit that referenced this issue Dec 6, 2022
fix crash due to nil msg.Ctx. Fixes #177
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant