{"action":"opened","issue":{"url":"https://api.github.com/repos/reactiveui/ReactiveUI/issues/1854","repository_url":"https://api.github.com/repos/reactiveui/ReactiveUI","labels_url":"https://api.github.com/repos/reactiveui/ReactiveUI/issues/1854/labels{/name}","comments_url":"https://api.github.com/repos/reactiveui/ReactiveUI/issues/1854/comments","events_url":"https://api.github.com/repos/reactiveui/ReactiveUI/issues/1854/events","html_url":"https://github.com/reactiveui/ReactiveUI/issues/1854","id":388740005,"node_id":"MDU6SXNzdWUzODg3NDAwMDU=","number":1854,"title":"Execution of ReactiveCommand throws sometimes an ObjectDisposedException","user":{"login":"Yarith","id":7803088,"node_id":"MDQ6VXNlcjc4MDMwODg=","avatar_url":"https://avatars3.githubusercontent.com/u/7803088?v=4","gravatar_id":"","url":"https://api.github.com/users/Yarith","html_url":"https://github.com/Yarith","followers_url":"https://api.github.com/users/Yarith/followers","following_url":"https://api.github.com/users/Yarith/following{/other_user}","gists_url":"https://api.github.com/users/Yarith/gists{/gist_id}","starred_url":"https://api.github.com/users/Yarith/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Yarith/subscriptions","organizations_url":"https://api.github.com/users/Yarith/orgs","repos_url":"https://api.github.com/users/Yarith/repos","events_url":"https://api.github.com/users/Yarith/events{/privacy}","received_events_url":"https://api.github.com/users/Yarith/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-12-07T17:10:03Z","updated_at":"2018-12-07T17:10:03Z","closed_at":null,"author_association":"NONE","body":"**Do you want to request a *feature* or report a *bug*?**\r\n*bug*\r\n\r\n**What is the current behavior?**\r\nThrows an `ObjectDisposedException` if the executing command somehow disposes itself.\r\n\r\n**If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem**\r\nThis is a small program which causes the described `ObjectDisposedException`.\r\n```csharp\r\nusing ReactiveUI;\r\nusing ReactiveUI.Fody.Helpers;\r\nusing System;\r\nusing System.Diagnostics;\r\nusing System.Reactive;\r\nusing System.Reactive.Concurrency;\r\nusing System.Reactive.Disposables;\r\nusing System.Reactive.Linq;\r\nusing System.Reactive.Subjects;\r\nusing System.Threading.Tasks;\r\nusing System.Windows.Threading;\r\n\r\nclass Program\r\n{\r\n    static async Task Main(string[] args)\r\n    {\r\n        using (var manager = new CustomerManagerViewModel())\r\n        {\r\n            Debug.Assert(manager.OpenedCustomer == null);\r\n            if (await manager.NewCustomer.CanExecute.FirstOrDefaultAsync())\r\n            {\r\n                await manager.NewCustomer.Execute();\r\n            }\r\n            Debug.Assert(manager.OpenedCustomer != null);\r\n            if (await manager.OpenedCustomer.Leave.CanExecute.FirstOrDefaultAsync())\r\n            {\r\n                await manager.OpenedCustomer.Leave.Execute(); // this line causes ObjectDisposedException\r\n            }\r\n        }\r\n    }\r\n}\r\n\r\nclass CustomerManagerViewModel : ReactiveObject, IDisposable\r\n{\r\n    private readonly IDisposable serialDisposeLastOpenedCustomer;\r\n\r\n    public CustomerManagerViewModel()\r\n    {\r\n        CanLeave = true;\r\n\r\n        NewCustomer = ReactiveCommand.Create(() => { },\r\n            outputScheduler: ImmediateScheduler.Instance);\r\n        Subject<Unit> openedCustomerLeft = new Subject<Unit>();\r\n        NewCustomer\r\n            .Select(_ =>\r\n                new CustomerViewModel(\r\n                    canLeaveObservable: this.WhenAnyValue(x => x.CanLeave)))\r\n            .Merge(openedCustomerLeft.Select(_ => (CustomerViewModel)null))\r\n            .ToPropertyEx(this, x => x.OpenedCustomer, scheduler: ImmediateScheduler.Instance);\r\n\r\n        // Serial dispose last opened customer        \r\n        serialDisposeLastOpenedCustomer =\r\n            this.WhenAnyValue(x => x.OpenedCustomer)\r\n                .Select(customer =>\r\n                    customer == null ? Observable.Empty<Unit>()\r\n                    : Observable.Create<Unit>(_ => customer))\r\n                .Switch()\r\n                .Subscribe();\r\n\r\n        this.WhenAnyObservable(x => x.OpenedCustomer.Leave)\r\n            .Subscribe(openedCustomerLeft);\r\n    }\r\n\r\n    public CustomerViewModel OpenedCustomer { [ObservableAsProperty] get; }\r\n\r\n    [Reactive]\r\n    public bool CanLeave { get; set; }\r\n\r\n    public ReactiveCommand<Unit, Unit> NewCustomer { get; }\r\n\r\n    public void Dispose() => serialDisposeLastOpenedCustomer.Dispose();\r\n}\r\n\r\nclass CustomerViewModel : ReactiveObject, IDisposable\r\n{\r\n    public CustomerViewModel(IObservable<bool> canLeaveObservable)\r\n    {\r\n        if (canLeaveObservable == null)\r\n            throw new ArgumentNullException(nameof(canLeaveObservable));\r\n\r\n        Leave = ReactiveCommand.Create(() => { },\r\n            // Register external source as can execute to get a requirement for dispose\r\n            canExecute: canLeaveObservable,\r\n            outputScheduler: ImmediateScheduler.Instance);\r\n    }\r\n\r\n    public ReactiveCommand<Unit, Unit> Leave { get; }\r\n\r\n    public void Dispose() => Leave.Dispose();\r\n}\r\n```\r\n\r\nThe disposal of the `_executionInfo`-Subject on the line [ReactiveCommand.cs#L683](https://github.com/reactiveui/ReactiveUI/blob/502963d37c59766c601bf6ac5349c63a461b9eb6/src/ReactiveUI/ReactiveCommand/ReactiveCommand.cs#L683) switches the internal observable to an `DisposedObserver` which throws an `ObjectDisposedException` if something is getting called from it. In this case it is the line [ReactiveCommand.cs#L666](https://github.com/reactiveui/ReactiveUI/blob/502963d37c59766c601bf6ac5349c63a461b9eb6/src/ReactiveUI/ReactiveCommand/ReactiveCommand.cs#L666) which will call the `OnNext` method.\r\n\r\n\r\n**What is the expected behavior?**\r\nIf the execution of the command somehow disposes itself it should not throw on the [finally block](https://github.com/reactiveui/ReactiveUI/blob/502963d37c59766c601bf6ac5349c63a461b9eb6/src/ReactiveUI/ReactiveCommand/ReactiveCommand.cs#L666) of the execution.\r\n\r\n\r\n**What is the motivation / use case for changing the behavior?**\r\nI have upgraded our program from 7.4.0 to 9.4.1 and got on some places an `ObjectDisposedException`. It gets thrown on the line [ReactiveCommand.cs#L666](https://github.com/reactiveui/ReactiveUI/blob/502963d37c59766c601bf6ac5349c63a461b9eb6/src/ReactiveUI/ReactiveCommand/ReactiveCommand.cs#L666). \r\n\r\nWe have many places in our application which has a command that causes the container of the command to get disposed. For example: We have a back button which is bound to a `Leave`-ReactiveCommand this causes the surrounding ViewModel to dispose together with the `Leave`-ReactiveCommand. Also an important information: We use a dispatcher scheduler which will execute the output immediately if it is already on the dispatcher thread. This is the reason why the sample code uses the `ImmediateScheduler`.\r\n\r\n\r\n**Which versions of ReactiveUI, and which platform / OS are affected by this issue? Did this work in previous versions of ReativeUI? Please also test with the [latest stable](https://www.nuget.org/profiles/reactiveui) and [development snapshot](https://reactiveui.net/docs/getting-started/installation/nuget-packages/development-package-feed)**\r\nThis has worked with the version 7.4.0. After the installation of the 9.4.1 version this issue happened. Other versions were not tested.\r\n\r\n\r\n**Other information (e.g. stacktraces, related issues, suggestions how to fix)**\r\n### Possible workaround\r\nI figured out a possible workaround with the observation of the `IsExecuting`-Observable. Together with a `RefCountDisposable` i can prevent the disposal until the execution is completed.\r\n```csharp\r\nSingleAssignmentDisposable isExecutingSubscription = new SingleAssignmentDisposable();\r\nvar refCountDisposable = new RefCountDisposable(\r\n    new CompositeDisposable(isExecutingSubscription, reactiveCommand));\r\ndisposeAfterExecute = refCountDisposable;\r\nisExecutingSubscription.Disposable =\r\n    reactiveCommand.IsExecuting\r\n        .Select(isExecuting => \r\n            isExecuting\r\n            ? Observable.Create<Unit>(_ => refCountDisposable.GetDisposable())\r\n            : Observable.Empty<Unit>())\r\n        .Switch()\r\n        .Subscribe();\r\n```\r\nInstead disposing the `reactiveCommand` it will now dispose the `disposeAfterExecute`-Disposable.\r\n\r\n### Ideas for a fix\r\nI think this issue was not raised earlier because the `ReactiveCommand` is used most of the time with an deferred scheduler. Maybe the implementation relies on the deferred execution so it passes the finally block before the `Dispose` method gets called. \r\n1. One way would be to defer the disposal of the `_executionInfo` via an `RefCountDisposable`.\r\n2. Another way would be to mark the `ReactiveCommand` as disposed and do not call the `OnNext` method if it is the case.\r\n\r\nMaybe there are even better approaches than these above.\r\n"}}
{"actor":{"display_login":"Yarith"}}