-
Notifications
You must be signed in to change notification settings - Fork 709
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[FIXED] Deadlock when accessing subscriptions map on consumer (#1671)
This fixes an issue where a deadlock could occur when calling `Stop()` or `Drain()` on `ConsumeContext` or `MessagesContext` and then calling `Consume` or `Messages` immediately. Switched to using a type-safe implementation of `sync.Map` for subscriptions map instead of locking the whole consumer state. Additionally, changed the type of atomic flags from `uint32` to `atomic.UInt32` to avoid accidental non-atomic reads/writes. Signed-off-by: Piotr Piotrowski <[email protected]> --------- Signed-off-by: Piotr Piotrowski <[email protected]>
- Loading branch information
Showing
8 changed files
with
328 additions
and
138 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,25 @@ | ||
| module github.com/nats-io/nats.go | ||
|
|
||
| go 1.19 | ||
| go 1.21 | ||
|
|
||
| toolchain go1.22.5 | ||
|
|
||
| require ( | ||
| github.com/golang/protobuf v1.4.2 | ||
| github.com/klauspost/compress v1.17.8 | ||
| github.com/klauspost/compress v1.17.9 | ||
| github.com/nats-io/jwt v1.2.2 | ||
| github.com/nats-io/nats-server/v2 v2.10.16 | ||
| github.com/nats-io/nats-server/v2 v2.10.17 | ||
| github.com/nats-io/nkeys v0.4.7 | ||
| github.com/nats-io/nuid v1.0.1 | ||
| go.uber.org/goleak v1.3.0 | ||
| golang.org/x/text v0.15.0 | ||
| golang.org/x/text v0.16.0 | ||
| google.golang.org/protobuf v1.23.0 | ||
| ) | ||
|
|
||
| require ( | ||
| github.com/minio/highwayhash v1.0.2 // indirect | ||
| github.com/nats-io/jwt/v2 v2.5.7 // indirect | ||
| golang.org/x/crypto v0.23.0 // indirect | ||
| golang.org/x/sys v0.20.0 // indirect | ||
| golang.org/x/crypto v0.24.0 // indirect | ||
| golang.org/x/sys v0.21.0 // indirect | ||
| golang.org/x/time v0.5.0 // indirect | ||
| ) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| // Copyright 2024 The NATS Authors | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package syncx | ||
|
|
||
| import "sync" | ||
|
|
||
| // Map is a type-safe wrapper around sync.Map. | ||
| // It is safe for concurrent use. | ||
| // The zero value of Map is an empty map ready to use. | ||
| type Map[K comparable, V any] struct { | ||
| m sync.Map | ||
| } | ||
|
|
||
| func (m *Map[K, V]) Load(key K) (V, bool) { | ||
| v, ok := m.m.Load(key) | ||
| if !ok { | ||
| var empty V | ||
| return empty, false | ||
| } | ||
| return v.(V), true | ||
| } | ||
|
|
||
| func (m *Map[K, V]) Store(key K, value V) { | ||
| m.m.Store(key, value) | ||
| } | ||
|
|
||
| func (m *Map[K, V]) Delete(key K) { | ||
| m.m.Delete(key) | ||
| } | ||
|
|
||
| func (m *Map[K, V]) Range(f func(key K, value V) bool) { | ||
| m.m.Range(func(key, value any) bool { | ||
| return f(key.(K), value.(V)) | ||
| }) | ||
| } | ||
|
|
||
| func (m *Map[K, V]) LoadOrStore(key K, value V) (V, bool) { | ||
| v, loaded := m.m.LoadOrStore(key, value) | ||
| return v.(V), loaded | ||
| } | ||
|
|
||
| func (m *Map[K, V]) LoadAndDelete(key K) (V, bool) { | ||
| v, ok := m.m.LoadAndDelete(key) | ||
| if !ok { | ||
| var empty V | ||
| return empty, false | ||
| } | ||
| return v.(V), true | ||
| } | ||
|
|
||
| func (m *Map[K, V]) CompareAndSwap(key K, old, new V) bool { | ||
| return m.m.CompareAndSwap(key, old, new) | ||
| } | ||
|
|
||
| func (m *Map[K, V]) CompareAndDelete(key K, value V) bool { | ||
| return m.m.CompareAndDelete(key, value) | ||
| } | ||
|
|
||
| func (m *Map[K, V]) Swap(key K, value V) (V, bool) { | ||
| previous, loaded := m.m.Swap(key, value) | ||
| return previous.(V), loaded | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,152 @@ | ||
| // Copyright 2024 The NATS Authors | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package syncx | ||
|
|
||
| import ( | ||
| "testing" | ||
| ) | ||
|
|
||
| func TestMapLoad(t *testing.T) { | ||
| var m Map[int, string] | ||
| m.Store(1, "one") | ||
|
|
||
| v, ok := m.Load(1) | ||
| if !ok || v != "one" { | ||
| t.Errorf("Load(1) = %v, %v; want 'one', true", v, ok) | ||
| } | ||
|
|
||
| v, ok = m.Load(2) | ||
| if ok || v != "" { | ||
| t.Errorf("Load(2) = %v, %v; want '', false", v, ok) | ||
| } | ||
| } | ||
|
|
||
| func TestMapStore(t *testing.T) { | ||
| var m Map[int, string] | ||
| m.Store(1, "one") | ||
|
|
||
| v, ok := m.Load(1) | ||
| if !ok || v != "one" { | ||
| t.Errorf("Load(1) after Store(1, 'one') = %v, %v; want 'one', true", v, ok) | ||
| } | ||
| } | ||
|
|
||
| func TestMapDelete(t *testing.T) { | ||
| var m Map[int, string] | ||
| m.Store(1, "one") | ||
| m.Delete(1) | ||
|
|
||
| v, ok := m.Load(1) | ||
| if ok || v != "" { | ||
| t.Errorf("Load(1) after Delete(1) = %v, %v; want '', false", v, ok) | ||
| } | ||
| } | ||
|
|
||
| func TestMapRange(t *testing.T) { | ||
| var m Map[int, string] | ||
| m.Store(1, "one") | ||
| m.Store(2, "two") | ||
|
|
||
| var keys []int | ||
| var values []string | ||
| m.Range(func(key int, value string) bool { | ||
| keys = append(keys, key) | ||
| values = append(values, value) | ||
| return true | ||
| }) | ||
|
|
||
| if len(keys) != 2 || len(values) != 2 { | ||
| t.Errorf("Range() keys = %v, values = %v; want 2 keys and 2 values", keys, values) | ||
| } | ||
| } | ||
|
|
||
| func TestMapLoadOrStore(t *testing.T) { | ||
| var m Map[int, string] | ||
|
|
||
| v, loaded := m.LoadOrStore(1, "one") | ||
| if loaded || v != "one" { | ||
| t.Errorf("LoadOrStore(1, 'one') = %v, %v; want 'one', false", v, loaded) | ||
| } | ||
|
|
||
| v, loaded = m.LoadOrStore(1, "uno") | ||
| if !loaded || v != "one" { | ||
| t.Errorf("LoadOrStore(1, 'uno') = %v, %v; want 'one', true", v, loaded) | ||
| } | ||
| } | ||
|
|
||
| func TestMapLoadAndDelete(t *testing.T) { | ||
| var m Map[int, string] | ||
| m.Store(1, "one") | ||
|
|
||
| v, ok := m.LoadAndDelete(1) | ||
| if !ok || v != "one" { | ||
| t.Errorf("LoadAndDelete(1) = %v, %v; want 'one', true", v, ok) | ||
| } | ||
|
|
||
| v, ok = m.Load(1) | ||
| if ok || v != "" { | ||
| t.Errorf("Load(1) after LoadAndDelete(1) = %v, %v; want '', false", v, ok) | ||
| } | ||
|
|
||
| // Test that LoadAndDelete on a missing key returns the zero value. | ||
| v, ok = m.LoadAndDelete(2) | ||
| if ok || v != "" { | ||
| t.Errorf("LoadAndDelete(2) = %v, %v; want '', false", v, ok) | ||
| } | ||
| } | ||
|
|
||
| func TestMapCompareAndSwap(t *testing.T) { | ||
| var m Map[int, string] | ||
| m.Store(1, "one") | ||
|
|
||
| ok := m.CompareAndSwap(1, "one", "uno") | ||
| if !ok { | ||
| t.Errorf("CompareAndSwap(1, 'one', 'uno') = false; want true") | ||
| } | ||
|
|
||
| v, _ := m.Load(1) | ||
| if v != "uno" { | ||
| t.Errorf("Load(1) after CompareAndSwap = %v; want 'uno'", v) | ||
| } | ||
| } | ||
|
|
||
| func TestMapCompareAndDelete(t *testing.T) { | ||
| var m Map[int, string] | ||
| m.Store(1, "one") | ||
|
|
||
| ok := m.CompareAndDelete(1, "one") | ||
| if !ok { | ||
| t.Errorf("CompareAndDelete(1, 'one') = false; want true") | ||
| } | ||
|
|
||
| v, _ := m.Load(1) | ||
| if v != "" { | ||
| t.Errorf("Load(1) after CompareAndDelete = %v; want ''", v) | ||
| } | ||
| } | ||
|
|
||
| func TestMapSwap(t *testing.T) { | ||
| var m Map[int, string] | ||
| m.Store(1, "one") | ||
|
|
||
| v, loaded := m.Swap(1, "uno") | ||
| if !loaded || v != "one" { | ||
| t.Errorf("Swap(1, 'uno') = %v, %v; want 'one', true", v, loaded) | ||
| } | ||
|
|
||
| v, _ = m.Load(1) | ||
| if v != "uno" { | ||
| t.Errorf("Load(1) after Swap = %v; want 'uno'", v) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.