search

MetacoSA / NBitcoin

Comprehensive Bitcoin library for the .NET framework.
MIT License
1.86k stars 844 forks source link

Cancellation tokens & .NET 6 #1086

Closed kiminuo closed 2 years ago

kiminuo commented 2 years ago

I'm aware of the https://github.com/MetacoSA/NBitcoin/pull/1085#issuecomment-1039800049 comment but this PR is supposed to show what can be done and I'm not sure what preprocessor directives should be used for this.

Anyway, if this PR makes sense to you, please tell me how to move it forward.

kiminuo commented 2 years ago

Too bad I don't have the right to run CI checks:

1 workflow awaiting approval
First-time contributors need a maintainer to approve running workflows. [Learn more.](https://docs.github.com/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks)
kiminuo commented 2 years ago

@NicolasDorier Any opinion on this please?

molnard commented 2 years ago

Makes sense - it can improve UnboservedTaskException coming from NBitcoin in Wasabi?

kiminuo commented 2 years ago

it can improve UnboservedTaskException coming from NBitcoin in Wasabi?

I haven't spot any unobserved exception in this code.

molnard commented 2 years ago

it can improve UnboservedTaskException coming from NBitcoin in Wasabi?

I haven't spot any unobserved exception in this code.

I meant that when you abandon a task and that throws an exception that will be unobserved.

NicolasDorier commented 2 years ago

hey @kiminuo sorry, reviewing now!

NicolasDorier commented 2 years ago

I will edit this PR to use right #define, but I think that's a good idea

NicolasDorier commented 2 years ago

@kiminuo can you cherry-pick this commit: https://github.com/MetacoSA/NBitcoin/commit/81aca91c9429e68fa88f6049563b604b8bf929c6 ?

kiminuo commented 2 years ago

@kiminuo can you cherry-pick this commit: 81aca91 ?

Yes, done. Thanks.

kiminuo commented 2 years ago

@molnard

it can improve UnboservedTaskException coming from NBitcoin in Wasabi?

I haven't spot any unobserved exception in this code.

I meant that when you abandon a task and that throws an exception that will be unobserved.

Yes, that's true. What I meant to say was that I haven't spotted any source of unobserved exceptions with regard to WithCancellation or this PR.

But I believe there are unobserved exceptions thrown in NBitcoin as I suspect that, for example, this line https://github.com/MetacoSA/NBitcoin/blob/881ac6cc7e5ac36f44213389a4bfb0a2bfed136e/NBitcoin/Protocol/Node.cs#L1085

is a source of unobserved exceptions we see in Wasabi. I can create a PR for that too but it would certainly require more research on my side to know how to fix it correctly.

NicolasDorier commented 2 years ago

making new version