EventSubscription could be released with Dispose

[SebastianStehle]

I have seen that there is the open task to call dispose on the EventSubscription. We could just use the Dispose method to unsubscribe, e.g.

public IDisposable ObserveAfterTransaction(Action<AfterTransactionEvent> action);

Advantages:

1. Subscriptions are native resources anyway.
2. You get warning from code analyzer to dispose the resource.
3. The API is closer to .NET, e.g. Rx.Net
4. The API surface is smaller.

[LSViana]

Hi, @SebastianStehle!

I agree with the idea, just not sure how to implement them yet.

I'd like a solution that doesn't involve exposing anything different than we have today. This should probably be handled internally by the library objects (for example, saving a reference to the "unsubscribe" subject).

What do you think?

[SebastianStehle]

I noticed - by making the mistake in my application - that even such a simple case where you want to subscribe forever is also buggy:

var doc = new Doc();
var map = doc.Map("A");

map.ObserveDeep(() => {

});

• If you don't keep the subscription somewhere it will eventually be collected.
• You can also not keep it in the map, because nothing has a reference to the map.
• Therefore you have to store it in the doc. This is the only place that makes sense actually.

This is how I implemented it in C#:

The subscriptions are managed by a subscriber per event. If you have 10 .NET subscriber only one subscription is created to rust. it works similar to Reactive RefCount operator that converts a cold to a hot subscription.

https://github.com/SebastianStehle/ydotnet/blob/improve-output/YDotNet/Document/Doc.cs#L73

The subscriber also catches exceptions, which is super important to avoid any unpredictable problems in rust:
https://github.com/SebastianStehle/ydotnet/blob/main/YDotNet/Document/Events/EventPublisher.cs#L35

Furthermore the subscriber adds itself to a event manager that exists per doc, this keeps the subscription alive as long as the document lives:
https://github.com/SebastianStehle/ydotnet/blob/main/YDotNet/Document/Events/EventSubscriber.cs#L39

[LSViana]

Hi, @SebastianStehle!

About this:

I noticed - by making the mistake in my application - that even such a simple case where you want to subscribe forever is also buggy: (...)

You mean the Delegate instance will be collected and this will cause a NullReferenceException when the Rust code tries to invoke the callback, right?

If so, this scenario is already fixed in a different way as I mentioned in this comment. Your solution looks good to me as well, it's just a bit heavier because of the extra code to "intermediate" the callback process.

[SebastianStehle]

You mean the Delegate instance will be collected and this will cause a NullReferenceException when the Rust code tries to invoke the callback, right?

If it would be a NullReferenceException it would be great, but it is TargetInvocationException or something similar that cannot be catched.

If so, this scenario is already fixed in a different way as I mentioned in #44 (comment). Your solution looks good to me as well, it's just a bit heavier because of the extra code to "intermediate" the callback process.

Yes, your solution was also my first approach, but then you have to store the event subscription somewhere, even if you never unsubscribe. I made this mistake in my application code and then I changed it.

I actually built the subscriber for something else. The idea was to subscribe on the root types when they are created the first time and then use the event to mark on branches as deleted when they are removed from an array or map or so. Then I would have an open subscription anyway and could reuse that for the user subscription. But the array event does not expose the old values (yet?), so it does not work.

[LSViana]

Hi, @SebastianStehle!

Now that #55 is merged and the EventSubscription objects have been changed to IDisposable instances, can we close this issue?