fix(Lazy): release lock if supplier throws#66
Conversation
|
Warning Review limit reached
More reviews will be available in 50 minutes and 2 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughPull Request модифицирует класс ChangesГарантированное освобождение блокировки в Lazy
Sequence Diagram(s)sequenceDiagram
participant Test as LazyTest
participant Lazy as LazyT
participant Lock as ReentrantLock
Test->>Lazy: getOrCompute(supplier_success)
Lazy->>Lock: lock()
Lazy->>Lazy: maybeCompute(supplier)
Lazy->>Lock: unlock() in finally
Lazy-->>Test: value
Test->>Lazy: getOrCompute(supplier_exception)
Lazy->>Lock: lock()
Lazy->>Lazy: maybeCompute(supplier) throws
Lazy->>Lock: unlock() in finally
Lazy-->>Test: exception released lock
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
37dd448 to
8e7c8c3
Compare
|
Issues attributed to commits in this pull requestThis pull request was merged and Sentry observed the following issues:
|



Summary
Lazy.getOrCompute(Supplier)обёрнут вtry/finally: до фиксаlock.unlock()стоял послеmaybeCompute(supplier)без guard, и любое RuntimeException из supplier'а оставлялоlockзахваченным до смерти потока.Lazyшарит внешнийReentrantLockс другими операциями (напримерDocumentContext.acquireLocks/rebuild/clearSecondaryDataв bsl-language-server, гдеcomputeLock/diagnosticsLockпереиспользуются), все последующие попытки взять lock зависали навсегда. Поток-владелец возвращался в poolidleи держал lock пожизненно.Проявление
Стек у нас в проде показывал:
```
"doc-...ManagerModule.bsl-executor" waiting on condition
at DocumentContext.acquireLocks(DocumentContext.java:387)
at DocumentContext.rebuild(...)
```
Lock-owner —
text-document-service-...вSynchronousQueue.take()(т.е. idle worker в пуле). RC: предыдущий task взял lock черезLazy.getOrCompute, supplier бросил RuntimeException,unlock()пропущен.Test plan
LazyTest.supplierExceptionReleasesLock— послеIllegalStateExceptionиз supplier'аlock.isLocked() == false,getHoldCount() == 0.LazyTest.supplierExceptionReleasesSharedLockMultipleAttempts— два неудачных attempt'а подряд + успешный третий, lock корректно отпускается на каждом проходе.getOrComputeReturnsValue/getOrComputeCachesValue/clearAllowsRecompute/getReturnsNullBeforeCompute) — поведение без изменений.🤖 Generated with Claude Code