Implements IJobWorker.IAsyncDisposable #771
Conversation
| [Obsolete("Use DisposeAsync instead.", false)] | ||
| public void Dispose() | ||
| { | ||
| _ = DisposeAsync(); |
There was a problem hiding this comment.
❓️ should we wait here?
There was a problem hiding this comment.
The current implementation does not wait. This is what is problematic. But waiting an async method from a sync one is really not a good practice
There was a problem hiding this comment.
After reading the docs:
- https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose
- https://learn.microsoft.com/en-us/dotnet/api/system.iasyncdisposable?view=net-9.0
I thought we might want to write the methods a bit differently and make sure we close/dispose all resources more directly. Especially after the reformat we see that there several objects, we are not keeping the reference to dispose them correctly. Speaking of the _. https://github.com/camunda-community-hub/zeebe-client-csharp/blob/main/Client/Impl/Worker/JobWorker.cs#L107-L111
I guess we could simply call dispose on them, like on the Task thats starts the polling, etc.
But I guess we still need the cancelation of the source - and disposing of it
| _ = transformer.LinkTo(output); | ||
| var linkInputTransformer = input.LinkTo(transformer); | ||
| var linkTransformerOutput = transformer.LinkTo(output); | ||
|
|
There was a problem hiding this comment.
Track polling Task in order to await for its completion. Note that I do not want to cancel the continuation. Finally, dispose TPL block objects.
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Update the signature so that we can pass the cancellation token to SendAsync
|
|
||
| /// <inheritdoc /> | ||
| public async ValueTask DisposeAsync() | ||
| { |
There was a problem hiding this comment.
Cancel the source token which will stop the pollingTask. Note that I've changed the log because it's not an error and very annoying to see it in reports.
|
|
||
| linkInputTransformer.Dispose(); | ||
| linkTransformerOutput.Dispose(); |
There was a problem hiding this comment.
❓ Should we maybe dispose the objects in our dispose? But I guess this will not work in the error case.
2 participants
#769
There is a slight behavior difference as the isRunning flag will only be set when the worker is effectively closed.