Skip to content

Conversation

@CaringDev
Copy link
Contributor

@CaringDev CaringDev commented Jun 11, 2024

fixes #2264

As an alternative we could use await using var enumerator... but this would increase the difference between the overloads.

@JoshClose
Copy link
Owner

I went with the other PR since it fixed it in all 3 places.

@JoshClose JoshClose closed this Jun 1, 2025
@CaringDev
Copy link
Contributor Author

CaringDev commented Jun 2, 2025

@JoshClose the other PR is 'better' if you favor consistency over correctness / necessity, i.e. if you want to follow These methods should all be the same very strictly (which is difficult, given async enumerators are different). As I said

As IAsyncEnumerator implements IAsyncDisposable we should unconditionally await DisposeAsync.

meaning https://github.com/JoshClose/CsvHelper/pull/2297/files#diff-e2f1fdf3fb99b4d4e65c30f3dc27dff37d93a39a2d8b1f1257f7685f3a97b5b7R638 should not check for neither IAsyncDisposable nor IDisposable and really just be https://github.com/JoshClose/CsvHelper/pull/2265/files#diff-e2f1fdf3fb99b4d4e65c30f3dc27dff37d93a39a2d8b1f1257f7685f3a97b5b7L629.
Also, enumerators implementing IAsyncDisposable but not IAsyncEnumerable should not be a thing, so the two other checks are superfluous.
The most important thing however is that the bug is fixed, so I'm happy anyways :-)

@JoshClose
Copy link
Owner

I totally read this wrong. Must have been tired. You're definitely correct here.

@JoshClose JoshClose reopened this Jun 2, 2025
@JoshClose JoshClose merged commit 2be6b6e into JoshClose:master Jun 2, 2025
@CaringDev CaringDev deleted the patch-1 branch June 2, 2025 21:14
@CaringDev
Copy link
Contributor Author

Hi @JoshClose

Sorry to bug you again... I think #2344 would be even better.

As an alternative we could use await using var enumerator... instead of finally but this would increase the difference between the overloads even more.

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.

CsvWriter.WriteRecordsAsync does not dispose e.g. EF Core AsyncEnumerators

2 participants