dotnet / dotnet/reactive

Potential problem in Observable.Using

Open
#1,738 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C#
Stars
7.2k
Forks
798
PR merge metrics
No merged PRs in 30d

Description

It appears that Observable.Using does not dispose of the instance created by the resourceFactory argument in the event that the observable created by the observableFactory throws (directly or indirectly).

The following test case demonstrates the issue.

```

[Theory]
[InlineData(true)]
[InlineData(false)]
public void TestCase(bool raisedException)
{
var disposeInvoked = false;
var onNextInvoked = false;
var onErrorInvoked = false;
var onCompletedInvoked = false;

var obs =
Observable.Using(
() => Disposable.Create(() => disposeInvoked = true),
d => Observable.Create(o =>
{
if (raisedException)
{
o.OnError(new Exception());
}
else
{
o.OnCompleted();
}
return Disposable.Empty;
});
});

Assert.Equal(disposeInvoked, false);

try
{
obs.Subscribe(
o => onNextInvoked = true,
e =>
{
onErrorInvoked = true;
throw e;
},
() => onCompletedInvoked = true);
}
catch
{
// ignored
}

Assert.Equal(onNextInvoked, false);
Assert.Equal(disposeInvoked, true);
Assert.Equal(onErrorInvoked, raisedException);
Assert.Equal(onCompletedInvoked, !raisedException);
}
```

I suspect that the problem can be fixed in Using.cs with the following change.

```
// It is important to set the disposable resource after
// Run(). In the synchronous case this would else dispose
// the the resource before the source subscription.
try
{
Run(source);
}
finally
{
_disposable.Disposable = disposable;
}
```

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.