Skip to content

Add warning log if ReentrantShortLock is held too long #6186

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 6 commits into from
Closed

Add warning log if ReentrantShortLock is held too long #6186

wants to merge 6 commits into from

Conversation

JihwanByun
Copy link

Motivation:

ReentrantShortLock is designed for short-lived locking in non-blocking threads. However, because blocking calls inside this class are allowed via BlockHound.allowBlockingCallsInside(), long lock hold times are not detected by BlockHound. This can potentially hide contention issues or misuses.

This change introduces a mechanism to detect when a lock is held longer than expected and logs a warning to help developers identify such cases.

Modifications:

  • Added duration tracking in ReentrantShortLock using ThreadLocal.
  • Logs a warning if the lock is held longer than the configured threshold.
  • Threshold is configurable via the JVM option: -Dcom.linecorp.armeria.reentrantShortLockWarnThresholdMillis=<millis>.
  • Default threshold is set to 50ms.
  • Added flag integration using FlagsProvider, Flags, and DefaultFlagsProvider.

Result:

@CLAassistant
Copy link

CLAassistant commented Mar 31, 2025 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 31, 2025 •
edited
Loading

🔍 Build Scan® (commit: 054396a)

Job name Status Build Scan®
build-ubicloud-standard-16-jdk-8 ✅ https://ge.armeria.dev/s/dhfyb2mqbm6bg
build-ubicloud-standard-16-jdk-21-snapshot-blockhound ✅ https://ge.armeria.dev/s/qjwfhljs2e43w
build-ubicloud-standard-16-jdk-17-min-java-17-coverage ❌ (failure) https://ge.armeria.dev/s/naerqhkbytfxa
build-ubicloud-standard-16-jdk-17-min-java-11 ✅ https://ge.armeria.dev/s/kei5nah3nws4y
build-ubicloud-standard-16-jdk-17-leak ✅ https://ge.armeria.dev/s/2ut53lo72byf6
build-ubicloud-standard-16-jdk-11 ✅ https://ge.armeria.dev/s/vbflbv5i5ujdw
build-macos-latest-jdk-21 ✅ https://ge.armeria.dev/s/f6jwwqlwldhhc

@JihwanByun JihwanByun closed this Mar 31, 2025
@JihwanByun JihwanByun reopened this Mar 31, 2025
@JihwanByun JihwanByun closed this Mar 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ikhoon ikhoon Awaiting requested review from ikhoon ikhoon is a code owner

@jrhee17 jrhee17 Awaiting requested review from jrhee17 jrhee17 is a code owner

@minwoox minwoox Awaiting requested review from minwoox minwoox is a code owner

@trustin trustin Awaiting requested review from trustin trustin is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Report a ReentrantShortLock that holds a lock for a long time.
2 participants
@JihwanByun @CLAassistant