Skip to content

Commit 38708d3

Browse files
committed
Fix asyncErrorChannel deadlock on shutdown
Signed-off-by: WHOIM1205 <rathourprateek8@gmail.com>
1 parent ba2b601 commit 38708d3

File tree

5 files changed

+106
-1
lines changed

5 files changed

+106
-1
lines changed
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. receiver/otlp)
7+
component: otelcol
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Fix shutdown deadlock when async error channel blocks during fatal error reporting
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: []
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# Optional: The change log or logs in which this entry should be included.
21+
# e.g. '[user]' or '[user, api]'
22+
# Include 'user' if the change is relevant to end users.
23+
# Include 'api' if there is a change to a library API.
24+
# Default: '[user]'
25+
change_logs: [user]

.github/workflows/utils/cspell.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@
212212
"extensionz",
213213
"fanout",
214214
"fanoutconsumer",
215+
"fatalonshutdown",
215216
"featureflags",
216217
"featuregate",
217218
"featuregates",

otelcol/collector.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ func NewCollector(set CollectorSettings) (*Collector, error) {
144144
// Per signal.Notify documentation, a size of the channel equaled with
145145
// the number of signals getting notified on is recommended.
146146
signalsChannel: make(chan os.Signal, 3),
147-
asyncErrorChannel: make(chan error),
147+
asyncErrorChannel: make(chan error, 1),
148148
configProvider: configProvider,
149149
bc: bc,
150150
updateConfigProviderLogger: cc.SetCore,

otelcol/collector_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,70 @@ func TestCollectorReportError(t *testing.T) {
175175
assert.Equal(t, StateClosed, col.GetState())
176176
}
177177

178+
func TestCollectorShutdownWithFatalError(t *testing.T) {
179+
factories, err := nopFactories()
180+
require.NoError(t, err)
181+
182+
factory := newFatalOnShutdownExtensionFactory()
183+
factories.Extensions[factory.Type()] = factory
184+
185+
col, err := NewCollector(CollectorSettings{
186+
BuildInfo: component.NewDefaultBuildInfo(),
187+
Factories: func() (Factories, error) { return factories, nil },
188+
ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-fatalerror.yaml")}),
189+
})
190+
require.NoError(t, err)
191+
192+
wg := startCollector(context.Background(), t, col)
193+
194+
assert.Eventually(t, func() bool {
195+
return StateRunning == col.GetState()
196+
}, 2*time.Second, 200*time.Millisecond)
197+
198+
col.Shutdown()
199+
200+
done := make(chan struct{})
201+
go func() {
202+
wg.Wait()
203+
close(done)
204+
}()
205+
206+
select {
207+
case <-done:
208+
assert.Equal(t, StateClosed, col.GetState())
209+
case <-time.After(10 * time.Second):
210+
t.Fatal("collector shutdown deadlocked")
211+
}
212+
}
213+
214+
type fatalOnShutdownExtension struct {
215+
component.StartFunc
216+
host component.Host
217+
}
218+
219+
func (e *fatalOnShutdownExtension) Start(_ context.Context, host component.Host) error {
220+
e.host = host
221+
return nil
222+
}
223+
224+
func (e *fatalOnShutdownExtension) Shutdown(context.Context) error {
225+
componentstatus.ReportStatus(e.host, componentstatus.NewFatalErrorEvent(errors.New("shutdown error")))
226+
return nil
227+
}
228+
229+
func newFatalOnShutdownExtensionFactory() extension.Factory {
230+
return extension.NewFactory(
231+
component.MustNewType("fatalonshutdown"),
232+
func() component.Config {
233+
return &struct{}{}
234+
},
235+
func(context.Context, extension.Settings, component.Config) (extension.Extension, error) {
236+
return &fatalOnShutdownExtension{}, nil
237+
},
238+
component.StabilityLevelStable,
239+
)
240+
}
241+
178242
// NewStatusWatcherExtensionFactory returns a component.ExtensionFactory to construct a status watcher extension.
179243
func NewStatusWatcherExtensionFactory(
180244
onStatusChanged func(source *componentstatus.InstanceID, event *componentstatus.Event),
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
receivers:
2+
nop:
3+
4+
exporters:
5+
nop:
6+
7+
extensions:
8+
fatalonshutdown:
9+
10+
service:
11+
extensions: [fatalonshutdown]
12+
pipelines:
13+
traces:
14+
receivers: [nop]
15+
exporters: [nop]

0 commit comments

Comments
 (0)