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

Proposal: Disposables in CoreFx: SerialDisposable, CompositeDisposable, etc #19822

Open
clairernovotny opened this issue Jan 5, 2017 · 16 comments
Assignees
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-Meta
Milestone

Comments

@clairernovotny
Copy link
Member

Moving here from dotnet/reactive#309

Disposbles have a lot of uses in many libraries. Many libraries have created their own management and disposable aggregates to combine multiple disposable's. There might be an opportunity to move some of this logic in to CoreFX as a common library.

A few questions:
1. Does this belong in CoreFX at all?
2. Should it be in a new library, System.Disposables or does it fit into an existing library?

/cc @ghuntley @mattpodwysocki @RxDave 

@ButchersBoy
Copy link

ButchersBoy commented Jan 5, 2017

👍 to this. Disposable implementations are useful on their own, outside of a RX setup. I have libraries where I have purposefully avoided a dependency on RX, but want the various disposables, and have ended up duplicating them internally.

I think they should be "promoted".

@RxDave
Copy link

RxDave commented Jan 6, 2017

It may also be worth doing another round of optimization through the types. I noticed that often the disposable objects are a major source of memory consumption when using Rx extensively. Perhaps there's opportunities for internal improvements? E.g., coalescing CompositeDisposables to prevent multiple List instances floating around? Not sure what's actually possible here, but worth a look?

@jpierson
Copy link

jpierson commented Jan 6, 2017

I've created classes for this purpose is several projects before ever using RX and would readily use the proposed examples if they were made available.

@tarekgh
Copy link
Member

tarekgh commented Jan 6, 2017

@onovotny it will be helpful if you can have the initial proposal so we can start the discussion on the deign.

CC @KrzysztofCwalina @terrajobst

@clairernovotny
Copy link
Member Author

clairernovotny commented Jan 7, 2017

@tarekgh what needs to be there for the initial proposal? There are a few types already in existence in Rx.NET that have been well tested and complete from an API perspective. There may be room for internal optimizations as @RxDave suggests, but that's an implementation detail.

The types are here currently: https://github.com/Reactive-Extensions/Rx.NET/tree/v3.1.1/Rx.NET/Source/System.Reactive.Core/Reactive/Disposables

  • AnonymousDisposable
  • BooleanDisposable
  • CancellationDisposable
  • CompositeDisposable
  • ContextDisposable
  • DefaultDisposable
  • Disposable
  • MultipleAssignmentDisposable
  • RefCountDisposable
  • ScheduledDisposable
  • SerialDisposable
  • SingleAssignmentDisposable
  • StableCompositeDisposable
  • ICancelable

Thanks

@tarekgh
Copy link
Member

tarekgh commented Jan 7, 2017

thanks @onovotny

what needs to be there for the initial proposal?

we need to list all types/APIs we need to expose, something like:

    public sealed class AnonymousDisposable : ICancelable
    {
        public AnonymousDisposable(Action dispose)
        {
        }

        public bool IsDisposed
        {
        }

        public void Dispose()
        {
        }
    }

and mention briefly the value having these types and how is going to be used.

@clairernovotny
Copy link
Member Author

clairernovotny commented Jan 7, 2017

@tarekgh is there an easier way to do this given that this is already "approved" API in System.Reactive.Disposables? Do we need to copy the API of a dozen types in here (if yes, fine, just seems redundant given that all the types are already in the Rx.NET repo). By "approved," I mean that the Rx team already spent considerable time designing these and they've been stable for years. The types in the repo are also already documented.

This is more about "promoting" the existing types/API from System.Reactive.Core into somewhere within CoreFx.

@tarekgh
Copy link
Member

tarekgh commented Jan 7, 2017

@terrajobst could you please advise how we can proceed with this request?

@onovotny is there any other docs or pointers rather than the code for these types?

@clairernovotny
Copy link
Member Author

clairernovotny commented Jan 7, 2017

@RxDave
Copy link

RxDave commented Jan 7, 2017

In response to @tarekgh

and mention briefly the value having these types and how is going to be used.

The IDisposable interface (the disposable pattern) has been for quite some time ubiquitously used, in part, as a representation of any behavior that has a scoped lifetime. It's not just about freeing memory. The elegance of the using keyword in C# for synchronous operations, and the fact that so many things easily map onto the disposable pattern, has certainly helped with this trend I'm sure. But given the prevalence of asynchrony in modern programming, the disposable pattern has found yet another usage that's equally important. Rather than the using keyword, we often find ourselves having to store disposables in a field to be disposed at a later time, asynchronously. It turns out that there are some common patterns in this space and the disposable types provided by Rx reify them. They make the composition of disposables much simpler.

Although these types are first-class in Rx, where they're mostly used as composite subscriptions to observables, they're often used outside of Rx as well; e.g., for compositing a list of disposables together into a single disposable that can be disposed in a single call (especially useful when the order in which they are disposed matters, such as when releasing COM objects with dependencies, or when disposing has side effects), for exposing a disposable proxy before the object that actually needs to be disposed is created (such as the eager disposal of an object that will be created by awaiting an asynchronous operation), for disposing of one object and creating its replacement as an atomic operation (such as when an asynchronous request is canceled and replaced with another request, such that the result of the previous request, if any, will be discarded; e.g., when a user types in a search box, it cancels any previous search that may still be in progress and replaces it with the latest search operation instead), among other uses.

  • AnonymousDisposable
    Allows for creating an IDisposable implementation out of a side-effecting delegate. Perhaps this type shouldn't be exposed publicly; use the Disposable type instead (see below).
  • BooleanDisposable
    Associates the state of a disposable with a Boolean value. It's useful in synchronous scenarios or where race conditions are acceptable and you just need to check whether a disposable has been disposed as a part of being composed with other disposables.
  • CancellationDisposable
    Associates the state of a disposable with a CancellationToken. Very useful for interoperating with Task-based asynchrony; e.g., to compose the cancellation of asynchronous operations into a single disposable object. Once you have a CancellationDisposable representing one or more async operations, you can then use the other disposable types to compose them, in varying ways, with other objects and operations that share the same lifetime scope, ultimately hiding all of the complexity of cancellation behind a single call to Dispose.
  • CompositeDisposable
    Associates disposables that have the same scope into a mutable disposable list. One of the most commonly used disposable types.
  • ContextDisposable
    Associates a SynchronizationContext-affine disposable with its context and marshals calls to Dispose onto the context.
  • DefaultDisposable
    Internal type.
  • Disposable
    A static class that provides static factory methods for creating disposables from delegates. See AnonymousDisposable above for details.
  • MultipleAssignmentDisposable
    Represents a disposable that can be replaced multiple times with different underlying behaviors. It's useful for when you need to swap a disposable and simply discard the previous.
  • RefCountDisposable
    Registers each use of the disposable and only disposes of the underlying disposable when the reference count drops to 0. It's useful for when the side effects of disposing potentially need to be shared among multiple disposers, like when multiple windows are opened and share some disposable resource, and as each window closes it elects to dispose of the shared resource, but the resource must only be disposed when the final window closes.
  • ScheduledDisposable
    Associates an IScheduler-affine disposable with its scheduler and marshals calls to Dispose through the scheduler (similar to ContextDisposable). The scheduler implementation is specifically defined by Rx; however, it's possible that a case can be made to move Rx's schedulers platform to CoreFx as well.
  • SerialDisposable
    Similar to MultipleAssignmentDisposable, it represents a disposable that can be replaced multiple times with different underlying behaviors; however, the previous disposable is disposed upon replacement, atomically. It's useful for when you need to swap a disposable and ensure that the previous disposable is actually disposed, such as cancelling an ongoing async operation while replacing it with a fresh operation. One of the most commonly used disposable types.
  • SingleAssignmentDisposable
    Represents lazy assignment to support eager disposal. It's useful when the underlying disposable hasn't been created yet, but the consumer needs a reference to it now, and in fact may dispose it before it's actually created. One of the most commonly used disposable types.
  • StableCompositeDisposable
    Similar to CompositeDisposable, but read-only.
  • ICancelable
    Couples the IDisposable interface with a Boolean IsDisposed property. The simplest example is BooleanDisposable.

@clairernovotny clairernovotny changed the title Proposal: Disposables in CoreFx: SerialDisposable<T>, CompositeDisposable<T>, etc Proposal: Disposables in CoreFx: SerialDisposable, CompositeDisposable, etc Jan 8, 2017
@tarekgh
Copy link
Member

tarekgh commented Jan 9, 2017

great! thanks @RxDave and @onovotny I will mark this issue as ready for review then.

@jpierson
Copy link

After considering this further it makes more sense to me to separate these Disposable types into their own library package instead of trying to pull them into a core framework. My original reason for agreeing with the proposal was to be able to use these disposable patterns without requiring a dependency Reactive Extensions because they are useful on their own. Are there any other strong benefits to making these part of CoreFx?

@terrajobst
Copy link
Member

This is a great issue. We need to approach this space more holistically. I don't think we want to add all functionality like this (area that is large enough to warrant its own library but doesn't need to be part of the platform layer) to CoreFX. On the other hand, we don't want to a bunch of random extensions with inconsistent names on top either. It would be nice if had some sort of agreed up and still reviewed set of platform extensions/community projects that can sit above CoreFX. I'll take a stab at a proposal and share it later.

@terrajobst terrajobst self-assigned this Feb 7, 2017
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 5.0 milestone Jan 31, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@joperezr
Copy link
Member

@terrajobst looks like this one has been stale for a while. Is corefxlab a reasonable place for these kind of APIs? If not, would you suggest creating its own repo under dotnet, or now that we are getting some extensions in runtime repo perhaps we can find a good fit for these?

@joperezr joperezr removed the untriaged New issue has not been triaged by the area owner label Feb 28, 2020
@mbreckon
Copy link

I've been beginning to use Reactive Extensions recently and was pleasantly surprised to see the various xxxxDisposable types as I've previously had to write my own (albeit much simpler) when simplifying objects that use a number of IDisposable objects (e.g. 3D graphics in engineering applications). I came here to see if there were any plans to build these APIs into the system .NET libraries.

What would need to happen for this to become reality? Is there anything I can help out with?

@danmoseley danmoseley modified the milestones: 5.0.0, Future Aug 7, 2020
@joperezr joperezr added this to Needs triage in Triage POD for Meta, Reflection, etc via automation Jan 29, 2021
@joperezr joperezr moved this from Needs triage to Future in Triage POD for Meta, Reflection, etc Jan 29, 2021
@dazinator
Copy link

dazinator commented Apr 19, 2023

I am forever creating these two disposables in varius projects - and it's frustrating that I typically copy paste them around. I could create a nuget package of course but for just two classes.. and it's usually always at a point where I just don't have time, as I hit the need for these whils tin the middle of trying to glue stuff together - so dont have time to drop what I am doing and go create a dedicated nuget package projject and solve this properly - so the copy paste continues!

  • ActionOnDispose (e.g call an action delegate when dispose is called)
  • CompositeDisposable (as explained already)

I am not sure about the other RX disposables mentioned and how general purpose they are. I could see the cancellation token one pootentially being useful but -- With an ActionOnDispose it seems pretty trivial to capture and fire the cancellation token within the Action lambda.. I certainly think the above two disposables should be built in to the framework.

I also note that react's CompositeDisposable is mutable, where as I have only ever typically required immutable CompositeDisposable's which are a lot simpler in implementation - i.e a fixed array of the IDisposable's are passed into the construtor and this is fixed for the lifetime ofo the composite. I am not sure if it warrants having an immutable version plus a mutable one like RX's, or whether the mutable one should be used for all cases..

Finally there is now also IAsyncDisposable to coonsider in the same way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-Meta
Projects
No open projects
Development

No branches or pull requests