-
Notifications
You must be signed in to change notification settings - Fork 4
refactor(core): 优化核心组件的线程安全性和错误处理 #67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,7 +21,7 @@ public void SetUp() | |
| private EasyEvents _easyEvents = null!; | ||
|
|
||
| /// <summary> | ||
| /// 测试单参数事件的功能,验证事件能够正确接收并传递int类型参数 | ||
| /// 测试单参数事件的功能,验证事件能够正确接收并传递int类型参数 | ||
| /// </summary> | ||
| [Test] | ||
| public void Get_EventT_Should_Trigger_With_Parameter() | ||
|
|
@@ -38,7 +38,7 @@ public void Get_EventT_Should_Trigger_With_Parameter() | |
| } | ||
|
|
||
| /// <summary> | ||
| /// 测试双参数事件的功能,验证事件能够正确接收并传递int和string类型的参数 | ||
| /// 测试双参数事件的功能,验证事件能够正确接收并传递int和string类型的参数 | ||
| /// </summary> | ||
| [Test] | ||
| public void Get_EventTTK_Should_Trigger_With_Two_Parameters() | ||
|
|
@@ -59,4 +59,112 @@ public void Get_EventTTK_Should_Trigger_With_Two_Parameters() | |
| Assert.That(receivedInt, Is.EqualTo(100)); | ||
| Assert.That(receivedString, Is.EqualTo("hello")); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// 测试并发场景下GetOrAdd的线程安全性 | ||
| /// </summary> | ||
| [Test] | ||
| public void GetOrAdd_Should_Be_Thread_Safe() | ||
| { | ||
| const int threadCount = 10; | ||
| const int iterationsPerThread = 100; | ||
| var tasks = new Task[threadCount]; | ||
| var exceptions = new List<Exception>(); | ||
|
|
||
| for (var i = 0; i < threadCount; i++) | ||
| { | ||
| tasks[i] = Task.Run(() => | ||
| { | ||
| try | ||
| { | ||
| for (var j = 0; j < iterationsPerThread; j++) | ||
| { | ||
| var @event = _easyEvents.GetOrAddEvent<Event<int>>(); | ||
| Assert.That(@event, Is.Not.Null); | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| lock (exceptions) | ||
| { | ||
| exceptions.Add(ex); | ||
| } | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| Task.WaitAll(tasks); | ||
|
|
||
| Assert.That(exceptions, Is.Empty, $"并发测试中发生异常: {string.Join(", ", exceptions.Select(e => e.Message))}"); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// 测试并发场景下AddEvent的行为 | ||
| /// </summary> | ||
| [Test] | ||
| public void AddEvent_Should_Throw_When_Already_Registered() | ||
| { | ||
| _easyEvents.AddEvent<Event<int>>(); | ||
|
|
||
| Assert.Throws<ArgumentException>(() => _easyEvents.AddEvent<Event<int>>()); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// 测试并发场景下多个不同事件类型的注册 | ||
| /// </summary> | ||
| [Test] | ||
| public void Concurrent_Registration_Of_Different_Event_Types_Should_Work() | ||
| { | ||
| const int threadCount = 5; | ||
| var tasks = new Task[threadCount]; | ||
| var exceptions = new List<Exception>(); | ||
|
|
||
| // 每个线程注册不同类型的事件 | ||
| for (var i = 0; i < threadCount; i++) | ||
| { | ||
| var index = i; | ||
| tasks[i] = Task.Run(() => | ||
| { | ||
| try | ||
| { | ||
| switch (index) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| { | ||
| case 0: | ||
| _easyEvents.GetOrAddEvent<Event<int>>(); | ||
| break; | ||
| case 1: | ||
| _easyEvents.GetOrAddEvent<Event<string>>(); | ||
| break; | ||
| case 2: | ||
| _easyEvents.GetOrAddEvent<Event<bool>>(); | ||
| break; | ||
| case 3: | ||
| _easyEvents.GetOrAddEvent<Event<int, string>>(); | ||
| break; | ||
| case 4: | ||
| _easyEvents.GetOrAddEvent<Event<double>>(); | ||
| break; | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| lock (exceptions) | ||
| { | ||
| exceptions.Add(ex); | ||
| } | ||
|
Comment on lines
+151
to
+154
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| }); | ||
| } | ||
|
|
||
| Task.WaitAll(tasks); | ||
|
|
||
| Assert.That(exceptions, Is.Empty); | ||
|
|
||
| // 验证所有事件都已注册 | ||
| Assert.That(_easyEvents.GetEvent<Event<int>>(), Is.Not.Null); | ||
| Assert.That(_easyEvents.GetEvent<Event<string>>(), Is.Not.Null); | ||
| Assert.That(_easyEvents.GetEvent<Event<bool>>(), Is.Not.Null); | ||
| Assert.That(_easyEvents.GetEvent<Event<int, string>>(), Is.Not.Null); | ||
| Assert.That(_easyEvents.GetEvent<Event<double>>(), Is.Not.Null); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -183,4 +183,166 @@ public void ToString_Should_Return_Value_As_String() | |
|
|
||
| Assert.That(result, Is.EqualTo("42")); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// 测试并发场景下的属性值设置 | ||
| /// </summary> | ||
| [Test] | ||
| public void Concurrent_Value_Set_Should_Be_Thread_Safe() | ||
| { | ||
| var property = new BindableProperty<int>(0); | ||
| const int threadCount = 10; | ||
| const int iterationsPerThread = 100; | ||
| var tasks = new Task[threadCount]; | ||
| var exceptions = new List<Exception>(); | ||
|
|
||
| for (var i = 0; i < threadCount; i++) | ||
| { | ||
| var threadId = i; | ||
| tasks[i] = Task.Run(() => | ||
| { | ||
| try | ||
| { | ||
| for (var j = 0; j < iterationsPerThread; j++) | ||
| { | ||
| property.Value = threadId * iterationsPerThread + j; | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| lock (exceptions) | ||
| { | ||
| exceptions.Add(ex); | ||
| } | ||
|
Comment on lines
+213
to
+216
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| }); | ||
| } | ||
|
|
||
| Task.WaitAll(tasks); | ||
|
|
||
| Assert.That(exceptions, Is.Empty, $"并发测试中发生异常: {string.Join(", ", exceptions.Select(e => e.Message))}"); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// 测试并发场景下的事件注册和触发 | ||
| /// </summary> | ||
| [Test] | ||
| public void Concurrent_Register_And_Trigger_Should_Be_Thread_Safe() | ||
| { | ||
| var property = new BindableProperty<int>(0); | ||
| const int threadCount = 5; | ||
| var tasks = new Task[threadCount]; | ||
| var exceptions = new List<Exception>(); | ||
| var callCounts = new int[threadCount]; | ||
|
|
||
| for (var i = 0; i < threadCount; i++) | ||
| { | ||
| var threadId = i; | ||
| tasks[i] = Task.Run(() => | ||
| { | ||
| try | ||
| { | ||
| property.Register(value => { Interlocked.Increment(ref callCounts[threadId]); }); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| lock (exceptions) | ||
| { | ||
| exceptions.Add(ex); | ||
| } | ||
|
Comment on lines
+249
to
+252
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| }); | ||
| } | ||
|
|
||
| Task.WaitAll(tasks); | ||
|
|
||
| // 触发事件 | ||
| property.Value = 42; | ||
|
|
||
| Assert.That(exceptions, Is.Empty); | ||
| Assert.That(callCounts.Sum(), Is.EqualTo(threadCount), "所有注册的处理器都应该被调用"); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// 测试并发场景下的注册和取消注册 | ||
| /// </summary> | ||
| [Test] | ||
| public void Concurrent_Register_And_UnRegister_Should_Be_Thread_Safe() | ||
| { | ||
| var property = new BindableProperty<int>(0); | ||
| const int threadCount = 10; | ||
| const int iterationsPerThread = 50; | ||
| var tasks = new Task[threadCount]; | ||
| var exceptions = new List<Exception>(); | ||
|
|
||
| for (var i = 0; i < threadCount; i++) | ||
| { | ||
| tasks[i] = Task.Run(() => | ||
| { | ||
| try | ||
| { | ||
| for (var j = 0; j < iterationsPerThread; j++) | ||
| { | ||
| Action<int> handler = _ => { }; | ||
| property.Register(handler); | ||
| property.UnRegister(handler); | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| lock (exceptions) | ||
| { | ||
| exceptions.Add(ex); | ||
| } | ||
|
Comment on lines
+293
to
+296
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| }); | ||
| } | ||
|
|
||
| Task.WaitAll(tasks); | ||
|
|
||
| Assert.That(exceptions, Is.Empty); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// 测试并发场景下RegisterWithInitValue的线程安全性 | ||
| /// </summary> | ||
| [Test] | ||
| public void Concurrent_RegisterWithInitValue_Should_Be_Thread_Safe() | ||
| { | ||
| var property = new BindableProperty<int>(42); | ||
| const int threadCount = 10; | ||
| var tasks = new Task[threadCount]; | ||
| var exceptions = new List<Exception>(); | ||
| var receivedValues = new List<int>(); | ||
|
|
||
| for (var i = 0; i < threadCount; i++) | ||
| { | ||
| tasks[i] = Task.Run(() => | ||
| { | ||
| try | ||
| { | ||
| property.RegisterWithInitValue(value => | ||
| { | ||
| lock (receivedValues) | ||
| { | ||
| receivedValues.Add(value); | ||
| } | ||
|
Comment on lines
+326
to
+329
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| }); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| lock (exceptions) | ||
| { | ||
| exceptions.Add(ex); | ||
| } | ||
|
Comment on lines
+334
to
+337
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| }); | ||
| } | ||
|
|
||
| Task.WaitAll(tasks); | ||
|
|
||
| Assert.That(exceptions, Is.Empty); | ||
| Assert.That(receivedValues.Count, Is.EqualTo(threadCount)); | ||
| Assert.That(receivedValues.All(v => v == 42), Is.True, "所有初始值都应该是42"); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trying to acquire a lock on a local variable is error-prone as different threads may end up locking on different instances of the same variable. Instead, you should be locking on a class field that is preferably designated as
privateandreadonly, i.e. a dedicated object. Microsoft guidelines explicitly state that you should avoid locking on: