Skip to content

Conversation

@kprokopenko
Copy link
Collaborator

Fixes #1942

Problem

The String() method in internal/endpoint/endpoint.go was acquiring a read lock and then calling Address() which tried to acquire another read lock. This could cause a deadlock when another goroutine called Touch() (which requires a write lock) between the two lock acquisitions.

Solution

The fix inlines the address resolution logic in String() to avoid the nested lock acquisition. This ensures that only one read lock is acquired per call to String().

Testing

  • Code compiles successfully
  • No linter errors
  • The fix follows the same logic as the original Address() method but without the nested locking

References

@github-actions
Copy link

summary

Inferred base version: v3.118.3
Suggested version: v3.118.4

@kprokopenko kprokopenko force-pushed the fix/endpoint-deadlock-1942 branch from 7c3c28e to f31158c Compare November 24, 2025 16:12
Fixes #1942

The String() method was acquiring a read lock and then calling Address()
which tried to acquire another read lock. This could cause a deadlock
when another goroutine called Touch() (which requires a write lock)
between the two lock acquisitions.

The fix inlines the address resolution logic in String() to avoid the
nested lock acquisition.
@codecov-commenter
Copy link

codecov-commenter commented Nov 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.62%. Comparing base (b09f8ed) to head (3096517).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1944      +/-   ##
==========================================
- Coverage   74.00%   72.62%   -1.38%     
==========================================
  Files         392      392              
  Lines       34343    34345       +2     
==========================================
- Hits        25414    24943     -471     
- Misses       7810     8261     +451     
- Partials     1119     1141      +22     
Flag Coverage Δ
experiment ?
go-1.21.x 72.60% <100.00%> (-0.13%) ⬇️
go-1.25.x 72.10% <100.00%> (-1.87%) ⬇️
integration 54.78% <100.00%> (-0.56%) ⬇️
macOS 47.20% <66.66%> (-0.03%) ⬇️
ubuntu 72.61% <100.00%> (-1.39%) ⬇️
unit 47.22% <66.66%> (-0.01%) ⬇️
windows 47.20% <66.66%> (-0.03%) ⬇️
ydb-24.4 53.99% <100.00%> (-0.32%) ⬇️
ydb-25.2 54.77% <100.00%> (-0.47%) ⬇️
ydb-latest ?
ydb-nightly ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kprokopenko kprokopenko merged commit 359cf5f into master Nov 25, 2025
36 checks passed
@kprokopenko kprokopenko deleted the fix/endpoint-deadlock-1942 branch November 25, 2025 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Deadlock in Endpoint struct

4 participants