Fix lock order inversion when leaving room / publishing room sessions.

Deadlock could happen between

1 @ 0x44038e 0x451898 0x45186f 0x46f325 0x489f3d 0xbb7b76 0xbb7b45 0xc1fe52 0xc190f7 0x473461
0x46f324	sync.runtime_SemacquireMutex+0x24							/usr/lib/go-1.21/src/runtime/sema.go:77
0x489f3c	sync.(*Mutex).lockSlow+0x15c								/usr/lib/go-1.21/src/sync/mutex.go:171
0xbb7b75	sync.(*Mutex).Lock+0x55									/usr/lib/go-1.21/src/sync/mutex.go:90
0xbb7b44	github.com/strukturag/nextcloud-spreed-signaling.(*ClientSession).RoomSessionId+0x24	/build/nextcloud-spreed-signaling-1.2.3/clientsession.go:157
0xc1fe51	github.com/strukturag/nextcloud-spreed-signaling.(*Room).publishActiveSessions+0x231	/build/nextcloud-spreed-signaling-1.2.3/room.go:925
0xc190f6	github.com/strukturag/nextcloud-spreed-signaling.(*Room).run+0x36			/build/nextcloud-spreed-signaling-1.2.3/room.go:179

(which locks "mu" in the room and then "mu" in the client session) and

1 @ 0x44038e 0x451898 0x45186f 0x46f3e5 0x48b44a 0xc1ba76 0xbba37e 0xbe2aab 0xbdf8e5 0xbee0f8 0xbb6134 0x473461
0x46f3e4	sync.runtime_SemacquireRWMutex+0x24							/usr/lib/go-1.21/src/runtime/sema.go:87
0x48b449	sync.(*RWMutex).Lock+0x69								/usr/lib/go-1.21/src/sync/rwmutex.go:152
0xc1ba75	github.com/strukturag/nextcloud-spreed-signaling.(*Room).RemoveSession+0x35		/build/nextcloud-spreed-signaling-1.2.3/room.go:440
0xbba37d	github.com/strukturag/nextcloud-spreed-signaling.(*ClientSession).LeaveRoom+0xdd	/build/nextcloud-spreed-signaling-1.2.3/clientsession.go:489
0xbe2aaa	github.com/strukturag/nextcloud-spreed-signaling.(*Hub).processRoom+0x6a		/build/nextcloud-spreed-signaling-1.2.3/hub.go:1268
0xbdf8e4	github.com/strukturag/nextcloud-spreed-signaling.(*Hub).processMessage+0x984		/build/nextcloud-spreed-signaling-1.2.3/hub.go:909
0xbee0f7	github.com/strukturag/nextcloud-spreed-signaling.(*Hub).OnMessageReceived+0x17		/build/nextcloud-spreed-signaling-1.2.3/hub.go:2427
0xbb6133	github.com/strukturag/nextcloud-spreed-signaling.(*Client).processMessages+0x53		/build/nextcloud-spreed-signaling-1.2.3/client.go:347

(which locks "mu" in the client session and then "mu" in the room).
This commit is contained in:
Joachim Bauch 2024-05-21 09:09:10 +02:00
parent f6125dac3f
commit dff78d0101
No known key found for this signature in database
GPG key ID: 77C1D22D53E15F02

View file

@ -69,10 +69,12 @@ type ClientSession struct {
mu sync.Mutex
client HandlerClient
room atomic.Pointer[Room]
roomJoinTime atomic.Int64
roomSessionId string
client HandlerClient
room atomic.Pointer[Room]
roomJoinTime atomic.Int64
roomSessionIdLock sync.RWMutex
roomSessionId string
publisherWaiters ChannelWaiters
@ -158,8 +160,8 @@ func (s *ClientSession) PublicId() string {
}
func (s *ClientSession) RoomSessionId() string {
s.mu.Lock()
defer s.mu.Unlock()
s.roomSessionIdLock.RLock()
defer s.roomSessionIdLock.RUnlock()
return s.roomSessionId
}
@ -403,8 +405,8 @@ func (s *ClientSession) SubscribeEvents() error {
}
func (s *ClientSession) UpdateRoomSessionId(roomSessionId string) error {
s.mu.Lock()
defer s.mu.Unlock()
s.roomSessionIdLock.Lock()
defer s.roomSessionIdLock.Unlock()
if s.roomSessionId == roomSessionId {
return nil
@ -433,8 +435,8 @@ func (s *ClientSession) UpdateRoomSessionId(roomSessionId string) error {
}
func (s *ClientSession) SubscribeRoomEvents(roomid string, roomSessionId string) error {
s.mu.Lock()
defer s.mu.Unlock()
s.roomSessionIdLock.Lock()
defer s.roomSessionIdLock.Unlock()
if err := s.events.RegisterRoomListener(roomid, s.backend, s); err != nil {
return err
@ -493,6 +495,9 @@ func (s *ClientSession) doUnsubscribeRoomEvents(notify bool) {
s.events.UnregisterRoomListener(room.Id(), s.Backend(), s)
}
s.hub.roomSessions.DeleteRoomSession(s)
s.roomSessionIdLock.Lock()
defer s.roomSessionIdLock.Unlock()
if notify && room != nil && s.roomSessionId != "" {
// Notify
go func(sid string) {