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

4.x: Inline disposability into ScheduledItem #561

Merged
merged 4 commits into from
Jun 6, 2018

Conversation

akarnokd
Copy link
Collaborator

This PR turns the abstract ScheduledItem into an IDisposable which allows direct cancellation when returned from a Schedule() method. The CurrentThreadScheduler had to wrap it before but I don't see any reason to not have it support dispose directly.

Also since this is a public API change, the API approval check required the expansion of the txt file with the altered signature.

@danielcweber
Copy link
Collaborator

Thought about this before, Even if this is not breaking, I don't know if we are going for public surface changes right now per roadmap.

@onovotny @ghuntley Could you please weigh in?

@danielcweber
Copy link
Collaborator

I'm thinking of merging this. First, there already were API surface changes from 4.0, second, we can't progress without touching the API and we're not retiring anything. However, could you please implement IDisposable explicitly to avoid confusion about Cancel/Dispose, I think this is good then.

@akarnokd
Copy link
Collaborator Author

akarnokd commented Jun 6, 2018

However, could you please implement IDisposable explicitly to avoid confusion about Cancel/Dispose, I think this is good then.

I don't follow. I've added IDisposable and public void Dispose() but can't remove Cancel as it is the part of the API surface.

@danielcweber
Copy link
Collaborator

No, don't remove Cancel, just have Dispose as explicit interface implementation. Instead

public void Dispose()

you write

void IDisposable.Dispose()

That hides Dispose on ScheduledItem. It will only be accessible if you explicitly cast it to IDisposable. Just to make sure there are no 2 confusing APIs doing the same. System.IO.Stream has it that way with Close and Dispose.

@danielcweber danielcweber merged commit 8a9a254 into dotnet:master Jun 6, 2018
@akarnokd akarnokd deleted the ScheduledItemEnhancements branch June 6, 2018 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants