Skip to content

Add Timeout to Object Get Disposal#1014

Merged
mtmk merged 3 commits into
mainfrom
obj-get-timeout
Dec 17, 2025
Merged

Add Timeout to Object Get Disposal#1014
mtmk merged 3 commits into
mainfrom
obj-get-timeout

Conversation

@mtmk

@mtmk mtmk commented Dec 11, 2025

Copy link
Copy Markdown
Member

@mtmk mtmk linked an issue Dec 11, 2025 that may be closed by this pull request
_msgChannel.Writer.TryComplete();

await _context.DeleteConsumerAsync(_stream, Consumer, _cancellationToken);
using var cts = new CancellationTokenSource(_opts.CleanupTimeout);

@MiloszKrajewski MiloszKrajewski Dec 11, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think combining both, timeout and original token, would be even better:

using var cts = CancellationTokenSource.CreateLinkedTokenSource(_cancellationToken);
cts.CancelAfter(_opts.CleanupTimeout);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair point. I can link them.

_msgChannel.Writer.TryComplete();

using var cts = new CancellationTokenSource(_opts.CleanupTimeout);
using var ctsLinked = CancellationTokenSource.CreateLinkedTokenSource(cts.Token, _cancellationToken);

@MiloszKrajewski MiloszKrajewski Dec 11, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't want to be preachy about it, but I cannot resist ;-)

This approach:

using var cts = CancellationTokenSource.CreateLinkedTokenSource(_cancellationToken);
cts.CancelAfter(_opts.CleanupTimeout);

creates one CancellationTokenSource and one (internal) Timer.
While this one:

using var cts = new CancellationTokenSource(_opts.CleanupTimeout);
using var ctsLinked = CancellationTokenSource.CreateLinkedTokenSource(cts.Token, _cancellationToken);

creates two CancellationTokenSources and one (internal) Timer.

The first one is just a little bit lighter resource-wise.
Also one less implicit finally.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! done

@mtmk
mtmk marked this pull request as ready for review December 12, 2025 12:10
@mtmk
mtmk requested a review from scottf December 12, 2025 12:10

@scottf scottf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mtmk
mtmk merged commit 6c2d026 into main Dec 17, 2025
21 checks passed
@mtmk
mtmk deleted the obj-get-timeout branch December 17, 2025 10:12
mtmk added a commit that referenced this pull request Dec 17, 2025
* Fix service hangs when stopping (#1016)
* Add Timeout to Object Get Disposal (#1014)
* Add validation for subjects (#1017)
@mtmk mtmk mentioned this pull request Dec 17, 2025
mtmk added a commit that referenced this pull request Dec 17, 2025
* Fix service hangs when stopping (#1016)
* Add Timeout to Object Get Disposal (#1014)
* Add validation for subjects (#1017)
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.

ObjectStore.GetAsync stuck for hours

3 participants