Skip to content
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

Stability fixes #1131

Merged
merged 30 commits into from
Jul 6, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
f729a28
Make the disposing of application context thread safe
helto4real Jul 4, 2024
3d2cdde
Fix the example app using get calendar events
helto4real Jul 4, 2024
116c321
Make the handling of cancellation more roboust to avoid the source to…
helto4real Jul 4, 2024
4b12ac0
added tests
helto4real Jul 4, 2024
1a39300
Fixed the throw instead
helto4real Jul 4, 2024
1418f0c
Ignore and tracelog the operation cancelled exception in background task
helto4real Jul 4, 2024
d3982a5
revert throwifcanceled #1
helto4real Jul 5, 2024
1df6c93
revert throwif cancelled #2
helto4real Jul 5, 2024
adb1913
Add the Flush feature on BackgroundTaskTracker
helto4real Jul 5, 2024
daedfa7
Fix the scheduler to be more robust and not call actions on disposed …
helto4real Jul 5, 2024
3a63fb7
Fix the code comment using interlocked instead
helto4real Jul 5, 2024
c4ce7ca
Fix warning and missing setting the disposed flag
helto4real Jul 5, 2024
6ac57d6
add test
helto4real Jul 5, 2024
dda52d0
Added context tests
helto4real Jul 5, 2024
ca0ab18
Added even more tests
helto4real Jul 5, 2024
61df6d5
add tests
helto4real Jul 5, 2024
72ba56d
fix test
helto4real Jul 5, 2024
1a01805
remove the last of throwifcancelled
helto4real Jul 5, 2024
98e002f
Add one more test
helto4real Jul 5, 2024
7af8423
renaming
helto4real Jul 5, 2024
3a597e6
Fix comments and from discussion
helto4real Jul 5, 2024
fc1296c
removed timing using TaskSource
helto4real Jul 5, 2024
9919c8e
using interlocked on more places
helto4real Jul 5, 2024
967ac9c
removed the disposable timer
helto4real Jul 5, 2024
8b7ad1b
fix last interlocked case
helto4real Jul 5, 2024
28119f4
forgott the test
helto4real Jul 5, 2024
1d39a59
revert back to volatile bools
helto4real Jul 6, 2024
78542ef
Added more checks for disposed and throws if so
helto4real Jul 6, 2024
13ef23a
added test for multople dispose
helto4real Jul 6, 2024
53bafb5
fix from comment
helto4real Jul 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/AppModel/NetDaemon.AppModel.Tests/AppModelTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,21 @@ public async Task TestGetApplicationsLocalWithDisposable()
app!.DisposeIsCalled.Should().BeTrue();
}

[Fact]
public async Task TestApplicationCanDisposeMultipleTimesWithoutExceptions()
{
// ARRANGE
// ACT
var loadApps = await TestHelpers.GetLocalApplicationsFromYamlConfigPath("Fixtures/Local");

// CHECK

// check the application instance is init ok
var application = loadApps.First(n => n.Id == "LocalApps.MyAppLocalAppWithDispose");
await application.DisposeAsync().ConfigureAwait(false);
await application.DisposeAsync().ConfigureAwait(false);
}

[Fact]
public async Task TestGetApplicationsLocalWithAsyncDisposable()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
using NetDaemon.AppModel.Internal;
using NetDaemon.AppModel.Internal.AppFactories;

namespace NetDaemon.AppModel.Tests.Context;

public class ApplicationContextTests
{
[Fact]
public async Task TestApplicationContectIsDisposedMultipleTimesNotThrowsException()
{
var serviceProvider = new ServiceCollection().BuildServiceProvider();
var appFactory = Mock.Of<IAppFactory>();
var applicationContext = new ApplicationContext(serviceProvider, appFactory);

await applicationContext.DisposeAsync();
await applicationContext.DisposeAsync();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ internal sealed class ApplicationContext : IAsyncDisposable
{
private readonly CancellationTokenSource _cancelTokenSource = new();
private readonly IServiceScope? _serviceScope;
private bool _isDisposed;
private int _isDisposed; // 0 = false, 1 = true

public ApplicationContext(IServiceProvider serviceProvider, IAppFactory appFactory)
{
Expand Down Expand Up @@ -34,10 +34,8 @@ public async Task InitializeAsync()

public async ValueTask DisposeAsync()
{
// prevent multiple Disposes because the Service Scope will also dispose this
if (_isDisposed) return;

_isDisposed = true;
if (Interlocked.CompareExchange(ref _isDisposed, 1, 0) == 1)
return;

if (!_cancelTokenSource.IsCancellationRequested)
await _cancelTokenSource.CancelAsync();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,47 @@ public void _SchedulePeriodicStopsAfterDisposeOfDisposableScheduler()
}


[Fact]
public void PeriodicSchedulerShouldNotCallActionIfItIsDisposedDuringSchedule()
{
var (inner, disposableScheduler) = CreateScheduler();

int called = 0;
using var _ = disposableScheduler.SchedulePeriodic(TimeSpan.FromMinutes(1), () => called++);

// Dispose before the time moves forward and trigger a schedule
disposableScheduler.Dispose();

inner.AdvanceBy(TimeSpan.FromMinutes(1).Ticks);
called.Should().Be(0);
}

[Fact]
public void PeriodicSchedulerShouldNotCallActionIfItIsDisposedBeforeScheduled()
{
var (inner, disposableScheduler) = CreateScheduler();

int called = 0;
// Dispose before the we call schedule
disposableScheduler.Dispose();
using var _ = disposableScheduler.SchedulePeriodic(TimeSpan.FromMinutes(1), () => called++);
inner.AdvanceBy(TimeSpan.FromMinutes(1).Ticks);
called.Should().Be(0);
}

[Fact]
public void SchedulerShouldNotCallActionIfItIsDisposedBeforeScheduled()
{
var (inner, disposableScheduler) = CreateScheduler();

int called = 0;
// Dispose before the we call schedule
disposableScheduler.Dispose();
using var _ = disposableScheduler.Schedule(() => called++);
inner.AdvanceBy(TimeSpan.FromMinutes(1).Ticks);
called.Should().Be(0);
}

[Fact]
public void SchedulePeriodicStopsAfterDisposeOfDisposableScheduler()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,23 @@ public void TestRunInCallsFunction()
Assert.True(isCalled);
}

[Fact]
public void TestRunInDoesNotCallFunctionIfDisposed()
{
// ARRANGE
var testScheduler = new TestScheduler();
bool isCalled = false;
var netDaemonScheduler = new NetDaemonScheduler(reactiveScheduler: testScheduler);
netDaemonScheduler.Dispose();
netDaemonScheduler.RunIn(TimeSpan.FromMinutes(1), () => isCalled = true);

// ACT
testScheduler.AdvanceBy(TimeSpan.FromMinutes(1).Ticks);

// ASSERT
Assert.False(isCalled);
}

[Fact]
public void TestRunInShouldNotCallFunctionIfNotDue()
{
Expand Down Expand Up @@ -91,6 +108,29 @@ public void TestRunAtCallsFunction()
Assert.True(isCalled);
}

[Fact]
public void TestRunAtNotCallFunctionIfDisposed()
{
// ARRANGE
var testScheduler = new TestScheduler();

// sets the date to a specific time so we do not get errors in UTC
var dueTime = new DateTime(2021, 1, 1, 0, 0, 0);
testScheduler.AdvanceTo(dueTime.Ticks);

bool isCalled = false;
var netDaemonScheduler = new NetDaemonScheduler(reactiveScheduler: testScheduler);

netDaemonScheduler.Dispose();
netDaemonScheduler.RunAt(testScheduler.Now.AddHours(1), () => isCalled = true);

// ACT and ASSERT
testScheduler.AdvanceBy(TimeSpan.FromMinutes(30).Ticks);
Assert.False(isCalled);
testScheduler.AdvanceBy(TimeSpan.FromMinutes(30).Ticks);
Assert.False(isCalled);
}

[Fact]
public void TestRunAtLogsException()
{
Expand Down Expand Up @@ -136,6 +176,63 @@ public void TestRunEveryCallsCorrectNrOfTimes()
Assert.Equal(5, nrOfTimesCalled);
}

[Fact]
public void TestRunEveryNotCallsIfDisposedBeforeSchedule()
{
// ARRANGE
var testScheduler = new TestScheduler();
// sets the date to a specific time so we do not get errors in UTC
var dueTime = new DateTime(2021, 1, 1, 0, 0, 0);
testScheduler.AdvanceTo(dueTime.Ticks);

int nrOfTimesCalled = 0;
var netDaemonScheduler = new NetDaemonScheduler(reactiveScheduler: testScheduler);

netDaemonScheduler.Dispose();
netDaemonScheduler.RunEvery(TimeSpan.FromSeconds(1), () => nrOfTimesCalled++);

// ACT and ASSERT
testScheduler.AdvanceBy(TimeSpan.FromSeconds(5).Ticks);
Assert.Equal(0, nrOfTimesCalled);
}

[Fact]
public void TestRunEveryCallsCorrectNrOfTimesIfDisposed()
{
// ARRANGE
var testScheduler = new TestScheduler();
// sets the date to a specific time so we do not get errors in UTC
var dueTime = new DateTime(2021, 1, 1, 0, 0, 0);
testScheduler.AdvanceTo(dueTime.Ticks);

int nrOfTimesCalled = 0;
var netDaemonScheduler = new NetDaemonScheduler(reactiveScheduler: testScheduler);

netDaemonScheduler.RunEvery(TimeSpan.FromSeconds(1), () => nrOfTimesCalled++);

// ACT and ASSERT
testScheduler.AdvanceBy(TimeSpan.FromSeconds(3).Ticks);
Assert.Equal(3, nrOfTimesCalled);
netDaemonScheduler.Dispose();
testScheduler.AdvanceBy(TimeSpan.FromSeconds(3).Ticks);
// No other calls thould been made on a disposed scheduler
Assert.Equal(3, nrOfTimesCalled);
}

[Fact]
public void SchedulerShouldNotThrowOnMultipleDisposeCalls()
{
// ARRANGE
var testScheduler = new TestScheduler();

var netDaemonScheduler = new NetDaemonScheduler(reactiveScheduler: testScheduler);

// ACT and ASSERT
// No exception should be thrown calling dispose multiple times
netDaemonScheduler.Dispose();
netDaemonScheduler.Dispose();
}

[Fact]
public void TestRunEveryLogsException()
{
Expand Down Expand Up @@ -181,6 +278,26 @@ public void TestRunEveryCallsCorrectNrOfTimesUsingForwardTime()
Assert.Equal(4, nrOfTimesCalled);
}

[Fact]
public void TestRunEveryNotCallsUsingForwardTimeWhenDisposed()
{
// ARRANGE
var testScheduler = new TestScheduler();
// sets the date to a specific time so we do not get errors in UTC
var dueTime = new DateTime(2021, 1, 1, 0, 0, 0);
testScheduler.AdvanceTo(dueTime.Ticks);

int nrOfTimesCalled = 0;
var netDaemonScheduler = new NetDaemonScheduler(reactiveScheduler: testScheduler);
netDaemonScheduler.Dispose();

netDaemonScheduler.RunEvery(TimeSpan.FromSeconds(1), testScheduler.Now.AddSeconds(2), () => nrOfTimesCalled++);

// ACT and ASSERT
testScheduler.AdvanceBy(TimeSpan.FromSeconds(5).Ticks);
Assert.Equal(0, nrOfTimesCalled);
}

[Fact]
public void TestRunEveryLogsExceptionUsingForwardTime()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,34 @@ namespace NetDaemon.Extensions.Scheduler;
/// <summary>
/// IScheduler decorator that will cancel pending scheduled tasks when Disposed
/// </summary>
public sealed class DisposableScheduler : IScheduler, IDisposable
/// <remarks>
/// Wraps a scheduler to make it cancel all pending subscriptions on dispose.
/// </remarks>
public sealed class DisposableScheduler(IScheduler scheduler) : IScheduler, IDisposable
{
private readonly IScheduler _innerScheduler;
private readonly IScheduler _innerScheduler = scheduler;
private readonly CancellationTokenSource _cancellationTokenSource = new();
private bool _isDisposed;

/// <summary>
/// Wraps a scheduler to make it cancel all pending subscriptions on dispose.
/// </summary>
public DisposableScheduler(IScheduler scheduler)
{
_innerScheduler = scheduler;
}

/// <inheritdoc />
public IDisposable Schedule<TState>(TState state, Func<IScheduler, TState, IDisposable> action)
{
if (_isDisposed) return Disposable.Empty;

return _innerScheduler.Schedule(state, action);
CancellationTokenRegistration? cancellationTokenRegistration = null;

var subscription = _innerScheduler.Schedule(state, (_, d) =>
{
if (_isDisposed) return Disposable.Empty;
// pass this to the action to make sure it will also use this scheduler to schedule subsequent tasks
var disposable = action(this, d);
cancellationTokenRegistration?.Unregister();
return disposable;
});

cancellationTokenRegistration = _cancellationTokenSource.Token.Register(() => subscription.Dispose());

return subscription;
}

/// <inheritdoc />
Expand All @@ -37,6 +45,7 @@ public IDisposable Schedule<TState>(TState state, TimeSpan dueTime, Func<ISchedu

var subscription = _innerScheduler.Schedule(state, dueTime, (_, d) =>
{
if (_isDisposed) return Disposable.Empty;
// pass this to the action to make sure it will also use this scheduler to schedule subsequent tasks
var disposable = action(this, d);
cancellationTokenRegistration?.Unregister();
Expand All @@ -57,6 +66,7 @@ public IDisposable Schedule<TState>(TState state, DateTimeOffset dueTime, Func<I

var subscription = _innerScheduler.Schedule(state, dueTime, (_, d) =>
{
if (_isDisposed) return Disposable.Empty;
// pass this to the action to make sure it will also use this scheduler to schedule subsequent tasks
var disposable = action(this, d);
cancellationTokenRegistration?.Unregister();
Expand All @@ -81,4 +91,4 @@ public void Dispose()
_isDisposed = true;
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System.Reactive.Disposables;

namespace NetDaemon.Extensions.Scheduler;

/// <summary>
Expand All @@ -8,7 +10,7 @@ public sealed class DisposableTimer : IDisposable
private readonly CancellationTokenSource _combinedToken;
private readonly CancellationTokenSource _internalToken;
private bool _disposed;

/// <summary>
/// Constructor
/// </summary>
Expand All @@ -19,6 +21,11 @@ public DisposableTimer(CancellationToken token)
_combinedToken = CancellationTokenSource.CreateLinkedTokenSource(_internalToken.Token, token);
}

/// <summary>
/// Empty disposable timer
/// </summary>
public static IDisposable Empty => Disposable.Empty;

/// <summary>
/// Token to use as cancellation
/// </summary>
Expand All @@ -35,4 +42,4 @@ public void Dispose()
_disposed = true;
}
}
}
}
Loading
Loading