mirror of
https://github.com/open-telemetry/opentelemetry-go.git
synced 2025-04-13 11:30:31 +02:00
Fix a possible nil-dereference crash (#478)
* Test for a panic inside global internal meter instrument's Unbind * Fix a possible nil-dereference crash There is a nil dereference crash if we perform some operations in certain order: - get a global meter - create an instrument - bind it - set the delegate - unbind the instrument - call some recording function on the not-really-bound-anymore instrument Unbind will run the no op run-once initialization routine, so the follow-up RecordOne call will not run it's initialization routine. Which RecordOne's initialization routine being skipped, the delegate to bounded instrument is not set, but the code is still trying to get a pointer to it and then unconditionally dereference it. Add an extra check for a nil pointer - if this is true, then Unbind was first and RecordOne should effectively be a no op. Co-authored-by: Joshua MacDonald <jmacd@users.noreply.github.com>
This commit is contained in:
parent
6b97bb047a
commit
29cd0c08b7
@ -233,6 +233,12 @@ func (bound *instHandle) RecordOne(ctx context.Context, number core.Number) {
|
||||
if implPtr == nil {
|
||||
implPtr = (*metric.BoundInstrumentImpl)(atomic.LoadPointer(&bound.delegate))
|
||||
}
|
||||
// This may still be nil if instrument was created and bound
|
||||
// without a delegate, then the instrument was set to have a
|
||||
// delegate and unbound.
|
||||
if implPtr == nil {
|
||||
return
|
||||
}
|
||||
(*implPtr).RecordOne(ctx, number)
|
||||
}
|
||||
|
||||
|
@ -225,3 +225,23 @@ func TestDefaultSDK(t *testing.T) {
|
||||
require.Equal(t, `{"updates":[{"name":"test.builtin{A=B}","sum":1}]}
|
||||
`, <-ch)
|
||||
}
|
||||
|
||||
func TestUnbindThenRecordOne(t *testing.T) {
|
||||
internal.ResetForTest()
|
||||
|
||||
// Note: this test uses oppsite Float64/Int64 number kinds
|
||||
// vs. the above, to cover all the instruments.
|
||||
ctx := context.Background()
|
||||
sdk := metrictest.NewProvider()
|
||||
meter := global.MeterProvider().Meter("test")
|
||||
counter := meter.NewInt64Counter("test.counter")
|
||||
boundC := counter.Bind(meter.Labels())
|
||||
global.SetMeterProvider(sdk)
|
||||
boundC.Unbind()
|
||||
|
||||
require.NotPanics(t, func() {
|
||||
boundC.Add(ctx, 1)
|
||||
})
|
||||
mock := global.MeterProvider().Meter("test").(*metrictest.Meter)
|
||||
require.Equal(t, 0, len(mock.MeasurementBatches))
|
||||
}
|
||||
|
Loading…
x
Reference in New Issue
Block a user