Skip to content

fix: ensure writer lock is released on exception in RefreshAuthToken#25

Open
mofiaboss wants to merge 1 commit into
Kount:masterfrom
mofiaboss:fix/issue-24-lock-release-on-exception
Open

fix: ensure writer lock is released on exception in RefreshAuthToken#25
mofiaboss wants to merge 1 commit into
Kount:masterfrom
mofiaboss:fix/issue-24-lock-release-on-exception

Conversation

@mofiaboss
Copy link
Copy Markdown

Summary

Fixes #24 - ReaderWriterLock deadlock when HTTP request fails during token refresh

Problem

When RefreshAuthToken() encounters an exception during the HTTP request (e.g., SSL failure, connection reset, timeout), the writer lock is never released. Since this is a static lock shared across all SDK instances, one failed refresh blocks ALL subsequent SDK calls until they timeout after 30 seconds.

This caused production incidents where cascading lock timeouts amplified transient network failures into extended outages.

Stack trace from production:

System.Threading.ReaderWriterLock+ReaderWriterLockApplicationException:
The operation has timed out. (Exception from HRESULT: 0x800705B4)
   at System.Threading.ReaderWriterLock.AcquireWriterLock(int millisecondsTimeout)
   at Kount.Ris.Request.RefreshAuthToken(string authUrl, string apiKey)

Solution

Wrap the method body in try/finally to ensure ReleaseWriterLock() is always called, regardless of success or failure:

private static void RefreshAuthToken(string authUrl, string apiKey)
{
    _bearerRefreshLock.AcquireWriterLock(TimeSpan.FromSeconds(30));
    try
    {
        // ... HTTP request and token parsing ...
    }
    finally
    {
        _bearerRefreshLock.ReleaseWriterLock();  // ALWAYS released
    }
}

Changes

  • Add try/finally block to guarantee lock release in RefreshAuthToken()
  • Remove redundant reader lock acquisition in the success path
  • Add comprehensive unit tests proving the lock is released after exceptions

Test Coverage

Added 4 new tests in RefreshAuthTokenLockReleaseTest.cs:

Test Validates
RefreshAuthToken_WhenConnectionRefused_ReleasesLockForNextCall Lock released after connection error
RefreshAuthToken_MultipleSequentialFailures_AllCompleteQuickly 5 calls complete in <10s (not 150s)
RefreshAuthToken_AfterException_CanImmediatelyAcquireLock Lock can be acquired immediately after exception
RefreshAuthToken_ConcurrentCalls_AllResolveWithinReasonableTime Concurrent calls don't permanently deadlock

All tests pass:

Test Run Successful.
Total tests: 4
     Passed: 4

Test Plan

  • All existing tests pass
  • New lock release tests pass
  • Verified fix handles connection refused scenario
  • Verified fix handles concurrent access scenario

🤖 Generated with Claude Code

Fixes #24 - ReaderWriterLock deadlock when HTTP request fails

Problem:
When RefreshAuthToken() encounters an exception during the HTTP request
(e.g., SSL failure, connection reset, timeout), the writer lock is never
released. Since this is a static lock shared across all SDK instances,
one failed refresh blocks ALL subsequent SDK calls until they timeout
after 30 seconds.

This caused production incidents where cascading lock timeouts amplified
transient network failures into extended outages.

Solution:
Wrap the method body in try/finally to ensure ReleaseWriterLock() is
always called, regardless of success or failure.

Changes:
- Add try/finally block to guarantee lock release
- Remove redundant reader lock acquisition in success path
- Add comprehensive unit tests proving lock is released after exceptions

The tests verify:
- Lock is released after connection failures
- Multiple sequential calls complete quickly (not 30s each)
- Lock can be immediately acquired after exception
- Concurrent calls all resolve without permanent contention

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inquiry constructor auth refresh errors

1 participant