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

sql: v23.1.1: nil pointer in getContentionEventInfo #104406

Closed
cockroach-teamcity opened this issue Jun 6, 2023 · 1 comment · Fixed by #106185
Closed

sql: v23.1.1: nil pointer in getContentionEventInfo #104406

cockroach-teamcity opened this issue Jun 6, 2023 · 1 comment · Fixed by #106185
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-sentry Originated from an in-the-wild panic report.

Comments

@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Jun 6, 2023

This issue was autofiled by Sentry. It represents a crash or reported error on a live cluster with telemetry enabled.

Sentry link: https://cockroach-labs.sentry.io/issues/4231557744/?referrer=webhooks_plugin

Panic message:

panic.go:884: runtime error: invalid memory address or nil pointer dereference
(1) attached stack trace
-- stack trace:
| runtime.gopanic
| GOROOT/src/runtime/panic.go:884
| runtime.panicmem
| GOROOT/src/runtime/panic.go:260
| runtime.sigpanic
| GOROOT/src/runtime/signal_unix.go:835
| github.com/cockroachdb/cockroach/pkg/sql.getContentionEventInfo
| github.com/cockroachdb/cockroach/pkg/sql/crdb_internal.go:7895
| github.com/cockroachdb/cockroach/pkg/sql.glob..func97.1
| github.com/cockroachdb/cockroach/pkg/sql/crdb_internal.go:7041
| github.com/cockroachdb/cockroach/pkg/sql.setupGenerator.func3
| github.com/cockroachdb/cockroach/pkg/sql/virtual_table.go:127
| github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx.func2
| github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:470
| runtime.goexit
| GOROOT/src/runtime/asm_amd64.s:1594
Wraps: (2) runtime error: invalid memory address or nil pointer dereference
Error types: (1) *withstack.withStack (2) runtime.errorString
-- report composition:
runtime.errorString
panic.go:884: *withstack.withStack (top exception)

Stacktrace (expand for inline code snippets):

GOROOT/src/runtime/panic.go#L883-L885 in runtime.gopanic
GOROOT/src/runtime/panic.go#L259-L261 in runtime.panicmem
GOROOT/src/runtime/signal_unix.go#L834-L836 in runtime.sigpanic

if dbDesc != nil {
schName = schemaDesc.GetName()
}
in pkg/sql.getContentionEventInfo
// https://github.com/cockroachdb/cockroach/issues/101826
schemaName, dbName, tableName, indexName, err := getContentionEventInfo(ctx, p, resp.Events[i])
if err != nil {
in pkg/sql.glob..func97.1
}
err := worker(ctx, funcRowPusher(addRow))
// Notify that we are done sending rows.
in pkg/sql.setupGenerator.func3
sp.UpdateGoroutineIDToCurrent()
f(ctx)
}()
in pkg/util/stop.(*Stopper).RunAsyncTaskEx.func2
GOROOT/src/runtime/asm_amd64.s#L1593-L1595 in runtime.goexit

GOROOT/src/runtime/panic.go in runtime.gopanic at line 884
GOROOT/src/runtime/panic.go in runtime.panicmem at line 260
GOROOT/src/runtime/signal_unix.go in runtime.sigpanic at line 835
pkg/sql/crdb_internal.go in pkg/sql.getContentionEventInfo at line 7895
pkg/sql/crdb_internal.go in pkg/sql.glob..func97.1 at line 7041
pkg/sql/virtual_table.go in pkg/sql.setupGenerator.func3 at line 127
pkg/util/stop/stopper.go in pkg/util/stop.(*Stopper).RunAsyncTaskEx.func2 at line 470
GOROOT/src/runtime/asm_amd64.s in runtime.goexit at line 1594
Tag Value
Cockroach Release v23.1.1
Cockroach SHA: 00f65ea
Platform linux amd64
Distribution CCL
Environment v23.1.1
Command server
Go Version ``
# of CPUs
# of Goroutines

Jira issue: CRDB-28514

@cockroach-teamcity cockroach-teamcity added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-sentry Originated from an in-the-wild panic report. labels Jun 6, 2023
@yuzefovich yuzefovich changed the title sentry: panic.go:884: runtime error: invalid memory address or nil pointer dereference (1) attached stack trace -- stack trace: | runtime.gopanic | GOROOT/src/runtime/panic.go:884 | runtime.panicmem ... sql: v23.1.1: nil pointer in getContentionEventInfo Jun 8, 2023
@yuzefovich
Copy link
Member

We're incorrectly checking dbDesc and not schemaDesc for non-nil.

@maryliag maryliag assigned zachlite and unassigned maryliag Jun 8, 2023
craig bot pushed a commit that referenced this issue Jul 5, 2023
105832: upgrades: try to repair catalog corruptions during upgrade r=postamar a=postamar

This change enhances the recently-added upgrade precondition check which
checks for catalog corruptions by trying to repair these automatically
when possible.

Fixes: #104425
Fixes: #85265

Release note (general change): upgrading the cluster version to a new
release will not only check for descriptor- and other catalog
corruptions but will attempt to repair some of them on a best-effort
basis. This should seamlessly get rid of all longstanding descriptor
back-reference corruptions, which typically don't manifest themselves
until a schema change or an upgrade are performed.

106185: sql: fix nil pointer dereference in crdb_internal.transaction_contention_events r=zachlite a=zachlite

`schemaDesc` was not checked for a nil pointer before dereferencing it. `schemaDesc` can be nil under the following conditions:

1. A schema descriptor can be deleted, in which case all of its tables would be deleted too.

2. The transaction used to retrieve the descriptor fails.

Given the structure of `getContentionEventInfo`, this nil pointer dereference is only possible under scenario 2: If a schema and its tables are deleted, a nil table descriptor would force an early return, and this code path would not execute.

Fixes: #104406
Epic: none
Release note: None

Co-authored-by: Marius Posta <[email protected]>
Co-authored-by: zachlite <[email protected]>
@craig craig bot closed this as completed in 1ca813e Jul 5, 2023
blathers-crl bot pushed a commit that referenced this issue Jul 5, 2023
…ion_events

schemaDesc was not checked for a nil pointer before dereferencing it.

schemaDesc can be nil under the following conditions:
1. A schema descriptor can be deleted, in which case all of its
tables would be deleted too.

2. The transaction used to retrieve the descriptor fails.

Given the structure of `getContentionEventInfo`, this nil pointer
dereference is only possible under scenario 2.

If a schema and its tables are deleted, a nil table descriptor
would force an early return, and this code path would not execute.

Fixes: #104406
Epic: none
Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-sentry Originated from an in-the-wild panic report.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants