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

util/tracing: panic in LogFields - runtime error: index out of range #25389

Closed
couchand opened this issue May 9, 2018 · 8 comments · Fixed by #26706
Closed

util/tracing: panic in LogFields - runtime error: index out of range #25389

couchand opened this issue May 9, 2018 · 8 comments · Fixed by #26706
Assignees
Labels
A-tracing Relating to tracing in CockroachDB. 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

@couchand
Copy link
Contributor

couchand commented May 9, 2018

https://sentry.io/cockroach-labs/cockroachdb/issues/552506410/

*log.safeError: stopper.go:175: runtime.errorString: runtime error: index out of range
  File "github.com/cockroachdb/cockroach/pkg/util/tracing/tracer_span.go", line 358, in LogFields
  File "github.com/cockroachdb/cockroach/pkg/util/log/trace.go", line 131, in eventInternal
  File "github.com/cockroachdb/cockroach/pkg/util/log/structured.go", line 153, in addStructured
  File "github.com/cockroachdb/cockroach/pkg/util/log/log.go", line 55, in logDepth
  File "github.com/cockroachdb/cockroach/pkg/util/log/log.go", line 98, in Warningf
...
(7 additional frame(s) were not displayed)

stopper.go:175: runtime.errorString: runtime error: index out of range
@couchand couchand added O-sentry Originated from an in-the-wild panic report. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-tracing Relating to tracing in CockroachDB. labels May 9, 2018
@couchand couchand changed the title util/tracing: *log.safeError: stopper.go:175: runtime.errorString: runtime error: index out of range util/tracing: panic in LogFields - runtime error: index out of range May 9, 2018
@knz
Copy link
Contributor

knz commented May 12, 2018

Sentry issue: COCKROACHDB-G1

@andreimatei
Copy link
Contributor

I've got no idea what's going on here. The line that panics is the Printf in

	if len(fields) == 1 && fields[0].Key() == "event" {
			s.netTr.LazyPrintf("%s", fields[0].Value())
		} 

I'm at a loss about how that can generate an index out of range, given that we just checked the length. The .Value() function also doesn't seem to be using any indexes.
And the caller is sp.LogFields(otlog.String("event", msg)), so there's no funny business there.

@RaduBerinde , do you happen to see anything here?

@tbg
Copy link
Member

tbg commented Jun 13, 2018 via email

@andreimatei
Copy link
Contributor

"Use after finish" of what exactly?

@RaduBerinde
Copy link
Member

My guess is that for whatever reason we're not seeing the top stack frames. The error is probably coming from inside net/trace where it is documented that crashes in that code are likely the result of use-after-free. See #26500 for another example of the same panic happening inside net/trace.

@andreimatei
Copy link
Contributor

Thanks. Ah, stupid sentry was hiding entries from the stack trace that was pasted here; interesting both at the top and the bottom. With this I can look again.

  File "github.com/cockroachdb/cockroach/vendor/golang.org/x/net/trace/trace.go", line 772, in addEvent
  File "github.com/cockroachdb/cockroach/vendor/golang.org/x/net/trace/trace.go", line 801, in LazyPrintf
  File "github.com/cockroachdb/cockroach/pkg/util/tracing/tracer_span.go", line 358, in LogFields
  File "github.com/cockroachdb/cockroach/pkg/util/log/trace.go", line 131, in eventInternal
  File "github.com/cockroachdb/cockroach/pkg/util/log/structured.go", line 153, in addStructured
  File "github.com/cockroachdb/cockroach/pkg/util/log/log.go", line 55, in logDepth
  File "github.com/cockroachdb/cockroach/pkg/util/log/log.go", line 98, in Warningf
  File "github.com/cockroachdb/cockroach/pkg/server/server.go", line 1462, in 2
  File "github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go", line 192, in func1

andreimatei added a commit to andreimatei/cockroach that referenced this issue Jun 13, 2018
We were using a context that was potentially long gone, leading to a
span use-after-free crash.

Fixes cockroachdb#25389

Release note (bug fix): Fixed a rare crash on node decomissioning.
craig bot pushed a commit that referenced this issue Jun 13, 2018
26706: server: use the right ctx when logging an error r=andreimatei a=andreimatei

We were using a context that was potentially long gone, leading to a
span use-after-free crash.

Fixes #25389

Release note (bug fix): Fixed a rare crash on node decomissioning.

Co-authored-by: Andrei Matei <[email protected]>
@solongordon
Copy link
Contributor

Sentry issue: COCKROACHDB-H2

@craig craig bot closed this as completed in #26706 Jun 13, 2018
@andreimatei
Copy link
Contributor

Two of the 3 sentry reports here did not belong and have not been fixed. Moved them to the new #26715.

andreimatei added a commit to andreimatei/cockroach that referenced this issue Jun 13, 2018
We were using a context that was potentially long gone, leading to a
span use-after-free crash.

Fixes cockroachdb#25389

Release note (bug fix): Fixed a rare crash on node decomissioning.
craig bot pushed a commit that referenced this issue Jun 14, 2018
26717: cherrypick 2.0: server: use the right ctx when logging an error r=andreimatei a=andreimatei

Cherry-pick of #26706

We were using a context that was potentially long gone, leading to a
span use-after-free crash.

Fixes #25389

Release note (bug fix): Fixed a rare crash on node decomissioning.

Co-authored-by: Andrei Matei <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tracing Relating to tracing in CockroachDB. 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.

6 participants