Skip to content

CBG-5031: fix panic for nil icebox map#84

Merged
bbrks merged 3 commits into
mainfrom
CBG-5031
Jan 6, 2026
Merged

CBG-5031: fix panic for nil icebox map#84
bbrks merged 3 commits into
mainfrom
CBG-5031

Conversation

@gregns1

@gregns1 gregns1 commented Jan 5, 2026

Copy link
Copy Markdown
Contributor

CBG-5031

Panic seems to happen when closing a sender's messages in icebox we seem to nil the icebox after. We should avoid this and just clear the messages in the icebox to avoid this panic.

@bbrks bbrks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Comment thread sender.go Outdated
if err := msg.Close(); err != nil {
sender.context.logFrame("Warning: Sender encountered error closing messages in icebox. Error: %v", err)
}
delete(sender.icebox, key)

@bbrks bbrks Jan 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could result in a build up of goroutines or messages again that will never be cleared from the icebox (see the godoc comment referring to CBG-4572)

We're unintentionally still guarded today by the panic when things get put in the icebox after a close.

I think it would still be preferable to nil the icebox map but add a nil check to requeue to early exit to avoid putting more things in it? Both functions have their locks to avoid races between the two.

@gregns1 gregns1 self-assigned this Jan 5, 2026
@gregns1 gregns1 assigned bbrks and unassigned gregns1 Jan 5, 2026
@bbrks bbrks merged commit 002c1b2 into main Jan 6, 2026
23 checks passed
@bbrks bbrks deleted the CBG-5031 branch January 6, 2026 11:36
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

Successfully merging this pull request may close these issues.

2 participants