Skip to content

Async locking#10

Closed
MarkCiliaVincenti wants to merge 1 commit intoNotOfficer:masterfrom
MarkCiliaVincenti:locking
Closed

Async locking#10
MarkCiliaVincenti wants to merge 1 commit intoNotOfficer:masterfrom
MarkCiliaVincenti:locking

Conversation

@MarkCiliaVincenti
Copy link
Copy Markdown
Contributor

No description provided.

@NotOfficer
Copy link
Copy Markdown
Owner

Hey, thanks for the PR but i think this isn't beneficial here. I'd rather not use async locking when the scope itself isn't async to reduce overload, especially in a progress callback.

@MarkCiliaVincenti
Copy link
Copy Markdown
Contributor Author

Hi, first off apologies for the indentation mess.
I see what you mean, but I wasn't sure if that callback call could take its time.
In essence the proposed code helps reduce mixing of synchronous code in async methods and the lock observes the cancellation token.

@NotOfficer
Copy link
Copy Markdown
Owner

Yeah no problem, i diffed it on a local tool with tabs replaced. Its an user-callback/event with the intention to just update a progress component's state. But i also see the pros of using the async lock if it might take longer. But then again you dont want a progress callback to take long, otherwise it will slowdown the download.

@NotOfficer NotOfficer closed this May 9, 2025
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