bug: Deadlock in Endpoint struct

[gyborisov]

Bug Report

YDB GO SDK version:

github.com/ydb-platform/ydb-go-sdk/v3 3.118.2

Environment

linux

Current behavior:

Reading from the topic stops. Goroutine dump shows that it stops because of a deadlock.

Expected behavior:

Reading from the topic does not stop due to this deadlock.

Steps to reproduce:

The deadlock can be reproduced in playground : https://go.dev/play/p/H5eeYf4ksQO

Related code:

Method String() in Endpoint (internal/endpoint/endpoint.go) acquires a read lock and then calls Address() method, which tries to acquire the same read lock in the same goroutine. If in some other goroutine method Touch() is called, and it happens that it calls Lock() in between of RLocks() in String(), a deadlock occures. This is what actually happened in our case.

func (e *endpoint) String() string {
	e.mu.RLock()
	defer e.mu.RUnlock()

	return fmt.Sprintf(`{id:%d,address:%q,local:%t,location:%q,loadFactor:%f,lastUpdated:%q}`,
		e.id,
		e.Address(), // <-- calls e.mu.RLock() in the same goroutine
		e.local,
		e.location,
		e.loadFactor,
		e.lastUpdated.Format(time.RFC3339),
	)
}

The deadlock can be reproduced in playground: https://go.dev/play/p/H5eeYf4ksQO

Other information:

Goroutine dumps:

Writer:

goroutine 26 [sync.RWMutex.Lock, 19 minutes]:
sync.runtime_SemacquireRWMutex(0x82e7d0?, 0xb0?, 0x816d00?)
	/-S/contrib/go/_std_1.25/src/runtime/sema.go:105 +0x25
sync.(*RWMutex).Lock(0xc01b6c5270?)
	/-S/contrib/go/_std_1.25/src/sync/rwmutex.go:155 +0x6b
github.com/ydb-platform/ydb-go-sdk/v3/internal/endpoint.(*endpoint).Touch(0xc00da41140, {0x0, 0x0, 0x815e40?})
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/endpoint/endpoint.go:179 +0x45
github.com/ydb-platform/ydb-go-sdk/v3/internal/balancer.(*Balancer).applyDiscoveredEndpoints(0xc01c3f7200, {0x80bc38, 0xc01b7a4cf0}, {0xc01b6c0a80, 0x6, 0x6}, {0xc01b6c2a70, 0x3})
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/balancer/balancer.go:213 +0x3c9
github.com/ydb-platform/ydb-go-sdk/v3/internal/balancer.(*Balancer).clusterDiscoveryAttempt(0xc01c3f7200, {0x80bc38, 0xc01b7a4cc0}, 0xc00034ac08)
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/balancer/balancer.go:172 +0x285
github.com/ydb-platform/ydb-go-sdk/v3/internal/balancer.(*Balancer).clusterDiscoveryAttemptWithDial(0xc01c3f7200, {0x80bc38, 0xc01b7a4c90})
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/balancer/balancer.go:139 +0x188
github.com/ydb-platform/ydb-go-sdk/v3/internal/repeater.(*repeater).wakeUp(0xc01c8b2230, {0x612e0c, 0x4})
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/repeater/repeater.go:162 +0x183
github.com/ydb-platform/ydb-go-sdk/v3/internal/repeater.(*repeater).worker.func3(...)
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/repeater/repeater.go:204
github.com/ydb-platform/ydb-go-sdk/v3/internal/repeater.(*repeater).worker(0xc01c8b2230, {0x80c070, 0xc00d8e9600}, {0x80af50, 0xc061efabd0})
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/repeater/repeater.go:217 +0x2f3
created by github.com/ydb-platform/ydb-go-sdk/v3/internal/repeater.New in goroutine 1
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/repeater/repeater.go:117 +0x2c5

Reader:

goroutine 414 [sync.RWMutex.RLock, 19 minutes]:
sync.runtime_SemacquireRWMutexR(0x0?, 0x1?, 0xc000053398?)
	/-S/contrib/go/_std_1.25/src/runtime/sema.go:100 +0x25
sync.(*RWMutex).RLock(...)
	/-S/contrib/go/_std_1.25/src/sync/rwmutex.go:74
github.com/ydb-platform/ydb-go-sdk/v3/internal/endpoint.(*endpoint).Address(0xc00da41140)
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/endpoint/endpoint.go:125 +0x59
github.com/ydb-platform/ydb-go-sdk/v3/internal/endpoint.(*endpoint).String(0xc00da41140)
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/endpoint/endpoint.go:90 +0x85
[...]
github.com/ydb-platform/ydb-go-sdk/v3/log.(*wrapper).Log(0x7fb727663f30?, {0x80bc38?, 0xc01b7a47e0?}, {0x63077a?, 0xc8fd38d15d2d3930?}, {0xc01b729900?, 0xc219d69216baa636?, 0x97d00455e87604c5?})
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/log/logger.go:138 +0x30
github.com/ydb-platform/ydb-go-sdk/v3/log.internalDriver.func7.1({{0x0, 0x0}, {0x0, 0x0, 0x0}, {0x0, 0x0}, {0x80a590, 0x82e7d0}, 0xc01b75e6c0})
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/log/driver.go:198 +0xafc
github.com/ydb-platform/ydb-go-sdk/v3/trace.(*Driver).Compose.func8.2({{0x0, 0x0}, {0x0, 0x0, 0x0}, {0x0, 0x0}, {0x80a590, 0x82e7d0}, 0xc01b75e6c0})
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/trace/driver_gtrace.go:314 +0xd5
github.com/ydb-platform/ydb-go-sdk/v3/trace.(*Driver).Compose.func8.2({{0x0, 0x0}, {0x0, 0x0, 0x0}, {0x0, 0x0}, {0x80a590, 0x82e7d0}, 0xc01b75e6c0})
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/trace/driver_gtrace.go:311 +0x98
github.com/ydb-platform/ydb-go-sdk/v3/trace.DriverOnConnInvoke.func1({0x0?, 0x0?}, {0x0?, 0xc000053ce0?, 0xc000053d30?}, {0x0?, 0xc000053d68?}, {0x80a590?, 0x82e7d0?}, 0xc01b75e6c0)
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/trace/driver_gtrace.go:1374 +0xb5
github.com/ydb-platform/ydb-go-sdk/v3/internal/conn.(*conn).Invoke.func1()
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/conn/conn.go:468 +0xd1
github.com/ydb-platform/ydb-go-sdk/v3/internal/conn.(*conn).Invoke(0xc00d611860, {0x80bc38, 0xc01b7a46f0}, {0x642d8f, 0x2b}, {0x5817e0, 0xc01b7a6150}, {0x4b6b80, 0xc01b7a45a0}, {0x0, ...})
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/conn/conn.go:491 +0x549
github.com/ydb-platform/ydb-go-sdk/v3/internal/balancer.(*Balancer).Invoke.func1({0x80bc38?, 0xc01b7a46f0?}, {0x815e40?, 0xc00d611860?})
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/balancer/balancer.go:349 +0x76
github.com/ydb-platform/ydb-go-sdk/v3/internal/balancer.(*Balancer).wrapCall(0xc01c3f7200, {0x80bc38, 0xc01b7a46f0}, 0xc0000540d0)
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/balancer/balancer.go:392 +0xff
github.com/ydb-platform/ydb-go-sdk/v3/internal/balancer.(*Balancer).Invoke(0x80bc38?, {0x80bc38?, 0xc01b7a46f0?}, {0x642d8f?, 0x2dff60?}, {0x5817e0?, 0xc01b7a6150?}, {0x4b6b80?, 0xc01b7a45a0?}, {0x0, ...})
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/balancer/balancer.go:348 +0xee
github.com/ydb-platform/ydb-go-sdk/v3.(*balancerWithMeta).Invoke(0xc000610870, {0x80bc38?, 0xc01b7a4600?}, {0x642d8f, 0x2b}, {0x5817e0, 0xc01b7a6150}, {0x4b6b80, 0xc01b7a45a0}, {0x0, ...})
	/-S/vendor/github.com/ydb-platform/ydb-go-sdk/v3/driver.go:128 +0xeb

[kprokopenko]

Thank you for reporting this deadlock issue! I've fixed it by refactoring the String() method to avoid nested lock acquisition.

The fix introduces a private getAddress() method that doesn't acquire a lock, which is used by both Address() and String() methods. This eliminates the deadlock while also removing code duplication.

Could you please review the PR #1944? I'd appreciate your feedback on the solution.

[gyborisov]

@kprokopenko Thank you!