Skip to content

fix(logging): Fix deadlock in deferred handler when creating loggers concurrently - #7632

Merged
kalleep merged 3 commits into
grafana:mainfrom
Vivek1898:fix/7625-logger-flush-deadlock
Nov 4, 2026
Merged

fix(logging): Fix deadlock in deferred handler when creating loggers concurrently#7632
kalleep merged 3 commits into
grafana:mainfrom
Vivek1898:fix/7625-logger-flush-deadlock

Conversation

@Vivek1898

@Vivek1898 Vivek1898 commented Nov 3, 2026 •
edited by kalleep
Loading

Copy link
Copy Markdown
Contributor

Brief description of Pull Request

WithAttrs and WithGroup on the deferred slog handler appended to the parent's children list under a read lock, so two components creating loggers at the same time could drop one child from the list. buildHandlers never reached that child; its buffered records were replayed by flushBuffer into addRecord, which waits on the lock flushBuffer holds, and Alloy hung in the logging block evaluation before any component started. The child registration now takes the write lock, and a child whose parent is already built derives its own handler on first use, so a missed child can no longer re-enter the buffer. Test: TestDeferredChildMissedByBuildIsBuiltFromParent detaches a child, buffers a record on it, and checks that Update returns and the record is written with its attributes.

Pull Request Details

Two layers on purpose: the write lock removes the lost-append race, and the self-build in realHandler keeps the flush from deadlocking if any other path ever leaves a child unbuilt.

Issue(s) fixed by this Pull Request

Fixes #7625

Notes to the Reviewer

PR Checklist

  • Documentation added
  • Tests updated
  • Config converters updated
  • This pull request was substantially generated with AI assistance (see the GenAI policy)

@Vivek1898
fix(logging): build a deferred child handler its parent build missed …
6aab6d9 
…instead of deadlocking the flush

WithAttrs and WithGroup appended to the parent's children list under a read
lock, so two components creating loggers at the same time could drop one
child from the list. buildHandlers never reached that child, its buffered
records were replayed by flushBuffer into addRecord, and addRecord waited
on the lock flushBuffer holds. Alloy then hung in the logging block
evaluation before any component started, about one restart in five.

Take the write lock when registering a child, and let a child whose
parent is already built derive its own handler on first use, so a missed
child can no longer re-enter the buffer.

Fixes grafana#7625
@Vivek1898
Vivek1898 requested a review from a team as a code owner November 3, 2026 21:25
@cla-assistant

cla-assistant Bot commented Nov 3, 2026 •
edited
Loading

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@cla-assistant

cla-assistant Bot commented Nov 3, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@kalleep

kalleep commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

This seems way to over-complicated to solve the linked issue.

Checking using read lock first and then upgrading to write lock and recheck should be enough to fix this issue e.g.

func (d *deferredSlogHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
	d.mut.RLock()
	if d.handle != nil {
		defer d.mut.RUnlock()
		return d.handle.WithAttrs(attrs)
	}
	d.mut.RUnlock()

	d.mut.Lock()
	defer d.mut.Unlock()

	// buildHandlers may have run since the read lock was released.
	if d.handle != nil {
		return d.handle.WithAttrs(attrs)
	}

	child := &deferredSlogHandler{
		attrs:    attrs,
		children: make([]*deferredSlogHandler, 0),
		l:        d.l,
	}
	d.children = append(d.children, child)
	return child
}

And then do the same for WithGroup

@kalleep kalleep self-assigned this Nov 4, 2026
kalleep
kalleep reviewed Nov 4, 2026
View reviewed changes
Comment thread internal/runtime/logging/deferred_handler_test.go Resolved
@Vivek1898
logging: take the write lock before registering a deferred child
83fffdc 
Check for a built handler under the read lock first, then upgrade to
the write lock and check again before appending the child, in both
WithAttrs and WithGroup. Drop the parent pointer and the self-build
path; with the append protected, buildHandlers reaches every child.

The test now registers children concurrently before Update and checks
that every one is built and replayed, instead of detaching a child by
hand.
kalleep
kalleep reviewed Nov 4, 2026
View reviewed changes
Comment thread internal/runtime/logging/deferred_handler_test.go Resolved
Comment thread internal/runtime/logging/deferred_handler.go Resolved
@kalleep kalleep changed the title fix(logging): Build a deferred child handler its parent build missed instead of deadlocking the flush fix(logging): Fix deadlock in deferred handler when creating loggers concurrently Nov 4, 2026
@kalleep kalleep added backport/v1.21 Backport to release/v1.21 and removed backport/v1.21 Backport to release/v1.21 labels Nov 4, 2026
@kalleep
kalleep merged commit 27d88b9 into grafana:main Nov 4, 2026
61 of 62 checks passed
@grafana-alloybot grafana-alloybot Bot mentioned this pull request Nov 4, 2026
Merged
jharvey10 pushed a commit that referenced this pull request Nov 13, 2026
@grafana-alloybot
chore(main): Release 1.22.0 (#7436)
3e1d273 
🤖 I have created a release *beep* *boop*
---


<details><summary>1.22.0</summary>

## [1.22.0](v1.21.0...v1.22.0)
(2026-11-12)


### ⚠ BREAKING CHANGES

* **loki.enrich:** Removes `target_match_label`, `logs_match_label` and
`metrics_match_label` from `loki.enrich` and `prometheus.enrich`. The
`target_to_log_match` and `target_to_metric_match` arguments are now
required.
* **otelcol.connector:** This may render certain configurations invalid.
These bad configurations were silently accepted before, but in practice
they never caused problems because all connectors currently defined only
output one signal type. This issue was surfaced while implementing
#7385, which supports more than one output signal.

### Features 🌟

* Add k8seventreceiver to the OTel Engine
([#7425](#7425))
([48317f3](48317f3))
* **component.http.oauth2:** Add jwt private key client authentication
([#7130](#7130))
([6fbbee3](6fbbee3))
* **database_observability.sql_server:** Add `server_configuration`
collector ([#7525](#7525))
([9d18f83](9d18f83))
* **database_observability.sql_server:** Add `server_metrics` collector
([#7545](#7545))
([f763183](f763183))
* **database_observability.sql_server:** Add additional performance
counter metrics to server_metrics
([#7596](#7596))
([a97d6a4](a97d6a4))
* **database_observability.sql_server:** Add throttling options to
query_samples collector
([#7567](#7567))
([8cfd0bd](8cfd0bd))
* **database_observability:** Add include settings for databases,
schemas, and users
([#7642](#7642))
([40be7ff](40be7ff))
* **database_observability:** Default
`enable_pre_classified_wait_events` to true
([#7579](#7579))
([21ac5ef](21ac5ef))
* **database_observability:** Emit parsed table names as a list with
op=query_parsed_table_names_v2
([#7520](#7520))
([67aaa9d](67aaa9d))
* **database_observability:** Only enable MySQL
perf_schema.eventsstatements collector by default in embedded prometheus
exporter ([#7514](#7514))
([d9e795f](d9e795f))
* **database_observability:** Preserve default exclusions and warn on
settings mismatch
([#7626](#7626))
([84a0021](84a0021))
* **helm:** Add distroless option
([#7516](#7516))
([962af92](962af92))
* **loki.enrich:** Add multi-label matching
([#7053](#7053))
([f168087](f168087))
* **loki.enrich:** Remove deprecated match arguments
([#7587](#7587))
([fbd2a29](fbd2a29))
* **otelcol.connector.exceptions:** Add exceptions connector component
from otel collector contrib
([#7385](#7385))
([782d4e3](782d4e3))
* **otelcol.receiver.kafka:** Add the `dial_timeout` argument (default
`10s`). Upstream now requires it. This also applies to
`otelcol.exporter.kafka`
([#7586](#7586))
([14ffd21](14ffd21))
* **otelcol.receiver.kafka:** Add the `max_decompress_batch_bytes`
argument (default `1073741824`)
([#7586](#7586))
([14ffd21](14ffd21))
* **otelcol.receiver.kafka:** Add the `max_prefetch` and `max_in_flight`
blocks to `partition_processing`
([#7586](#7586))
([14ffd21](14ffd21))
* **otelcol.receiver.kafka:** Deprecate `partition_processing >
max_buffered_batches` in favor of `partition_processing > max_prefetch >
batches`. Upstream removes it in v0.166.0
([#7586](#7586))
([14ffd21](14ffd21))
* **otelcol.receiver.kafka:** Support encoding extensions
([#7430](#7430))
([ede6371](ede6371))
* **otelcol:** Add Azure encoding extension
([#7400](#7400))
([146ed71](146ed71))
* **otelcol:** Upgrade to OTel Collector v0.164.1
([#7586](#7586))
([14ffd21](14ffd21))
* **prometheus.exporter.cadvisor:** Add housekeeping_interval arguments
([#7478](#7478))
([163eeef](163eeef))
* **prometheus.operator:** Support Prometheus operator scrape classes
([#6466](#6466))
([a941fb5](a941fb5))
* **prometheus.snmp_exporter:** Support multiple config files
([#6892](#6892))
([51f6ffb](51f6ffb))
* Promote loki.enrich and prometheus.enrich to public preview
([#7597](#7597))
([87cca81](87cca81))
* **pyroscope.ebpf:** Update ebpf-profiler fork to upstream v0.0.202640
([#7408](#7408))
([7b0591a](7b0591a))
* **pyroscope.java:** Update async-profiler to v4.6
([#7447](#7447))
([33bd346](33bd346))


### Bug Fixes 🐛

* **collector:** Update root x/net dependency to v0.62.0
([#7599](#7599))
([5c7b2a1](5c7b2a1))
* **database_observability.mysql:** Enable additional collectors on
embedded exporter
([#7608](#7608))
([8caea29](8caea29))
* **database_observability.mysql:** Omit digest_text label
([#7607](#7607))
([4ceaddf](4ceaddf))
* **database_observability.postgres:** Improve table registry lifecycle
and unqualified lookup
([#7470](#7470))
([587dab2](587dab2))
* **database_observability.postgres:** Include instance label in ID
generation ([#7301](#7301))
([295b6ce](295b6ce))
* **database_observability.sql_server:** Check server-level permissions
and avoid false positives in health check
([#7656](#7656))
([ae815a7](ae815a7))
* **database_observability.sql_server:** Include instance name in
generated `server_id`
([#7521](#7521))
([5e9571c](5e9571c))
* **database_observability.sql_server:** Parse TopSort and Spool
wrappers and keep subtrees of unmodeled operators
([#7406](#7406))
([2e887f6](2e887f6))
* **database_observability.sql_server:** Preserve query shape avoiding
normalization ([#7395](#7395))
([72bd5fc](72bd5fc))
* **database_observability.sql_server:** Report Azure SQL engine by name
in engine_version
([#7502](#7502))
([6760716](6760716))
* **database_observability.sql_server:** Support StmtCond explain plans
([#7598](#7598))
([c320b9d](c320b9d))
* **database_observability.sql_server:** Validate parsed table names
([#7413](#7413))
([7f06682](7f06682))
* **database_observability:** Apply exclusions to embedded exporters
([#7580](#7580))
([8753e61](8753e61))
* **deps:** Update github.com/andybalholm/brotli to v1.2.7
([#7519](#7519))
([fccef0f](fccef0f))
* **faro.receiver:** Enforce download_from_origins on redirects and
sourcemap URLs ([#7477](#7477))
([9ddd772](9ddd772))
* Hide HTTP proxy env vars in support bundles
([#7559](#7559))
([9a75d53](9a75d53))
* **logging:** Fix deadlock in deferred handler when creating loggers
concurrently ([#7632](#7632))
([27d88b9](27d88b9))
* **loki.process:** Move stage.logfmt to syntax.Validator
([#7446](#7446))
([aba9a0e](aba9a0e))
* **loki.process:** Move stage.luhn to syntax.Validator
([#7421](#7421))
([beeec13](beeec13))
* **loki.process:** Move stage.multiline to syntax.Validator
([#7420](#7420))
([095f423](095f423))
* **loki.process:** Move stage.replace to syntax.Validator
([#7518](#7518))
([2524277](2524277))
* **loki.source.cloudflare:** Stop additional_fields from corrupting the
extended and all field presets
([#7535](#7535))
([7fd4068](7fd4068))
* **loki.source.file:** Always update position
([#7641](#7641))
([81c690f](81c690f))
* **loki.source.file:** Read recreated files when the inode is reused
([#7538](#7538))
([e8a8e85](e8a8e85))
* **loki.source.kubernetes,loki.source.podlogs:** Prevent re-ingesting
logs on re-tails ([#7507](#7507))
([2db65de](2db65de))
* **loki.source.windowsevent:** Keep the saved bookmark when no new
event arrives after a restart
([#7589](#7589))
([b1cf13e](b1cf13e))
* **loki.write:** Keep the client running when its settings are
unchanged ([#7620](#7620))
([24064ae](24064ae))
* **loki.write:** WAL segment savepoints
([#7200](#7200))
([2dcc043](2dcc043))
* Migrate config from type string to type secret [backport]
([#7499](#7499))
([b89853a](b89853a))
* **mimir.alerts.kubernetes:** Pick up Secret changes without a restart
([#7273](#7273))
([0bc2b79](0bc2b79))
* **mimir.rules.kubernetes:** Fix non-recovering unhealthy state after a
failed restart when Mimir is unreachable
([#6339](#6339))
([8dd633d](8dd633d))
* **otelcol.connector:** Reject output signals that a connector cannot
produce ([#7389](#7389))
([5343639](5343639))
* **otelcol.exporter.otlphttp:** Deprecate enabled in the cookies block
([#7418](#7418))
([659c5eb](659c5eb))
* **otelcol.exporter.prometheus:** Pass missing `include_scope_labels`
to converter ([#6787](#6787))
([f0d65bd](f0d65bd))
* **otelcol.processor.tail_sampling:** Pass invert_match through for
boolean_attribute policies
([#7444](#7444))
([c139dd7](c139dd7))
* **otelcol:** Report full build version including pre-release suffix in
service.version ([#7523](#7523))
([e306584](e306584))
* **prometheus.exporter.kafka:** Avoid panic on empty consumer group
member assignment
([#7428](#7428))
([c49e0de](c49e0de))
* **prometheus.operator:** Initialize controller-runtime logging
([#7287](#7287))
([072feda](072feda))
* **prometheus.remote_write:** Write start timestamp zero samples to the
correct series ([#7633](#7633))
([5a12f45](5a12f45))
* **prometheus:** Apply relabel and enrich to start timestamp zero
samples ([#7647](#7647))
([035e298](035e298))
* **pyroscope.ebpf:** Bug with dropped partially symbolized samples.
([#7409](#7409))
([49b0764](49b0764))
* **remotecfg:** Restore last success metric in notModified response
([#7486](#7486))
([6eaa407](6eaa407))
* **security/HIGH/internal/web/ui:** Update npm dependency vite to
v7.3.7 [SECURITY]
([#7464](#7464))
([3a126f4](3a126f4))
* Update go version to 1.27.3
([#7469](#7469))
([8d0105f](8d0105f))
</details>

<details><summary>syntax: 0.1.3</summary>

##
[0.1.3](syntax/v0.1.2...syntax/v0.1.3)
(2026-11-12)


### Bug Fixes 🐛

* Update go version to 1.27.3
([#7469](#7469))
([8d0105f](8d0105f))
</details>

<details><summary>helm-chart: 1.16.0</summary>

##
[1.16.0](helm-chart/1.15.0...helm-chart/1.16.0)
(2026-11-12)


### Features 🌟

* **helm:** Update to Grafana Alloy v1.22.0
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: grafana-alloybot[bot] <167359181+grafana-alloybot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kalleep kalleep kalleep approved these changes

Assignees

@kalleep kalleep

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug: Self-deadlock in logging component setup (LoggingConfigNode.Evaluate) due to non-reentrant mutex during flushBuffer

2 participants

@Vivek1898 @kalleep