Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VSTHRD200 reports DisposeAsyncCore #805

Closed
MartyIX opened this issue Mar 15, 2021 · 4 comments · Fixed by #1238
Closed

VSTHRD200 reports DisposeAsyncCore #805

MartyIX opened this issue Mar 15, 2021 · 4 comments · Fixed by #1238
Assignees

Comments

@MartyIX
Copy link
Contributor

MartyIX commented Mar 15, 2021

Bug description

VSTHRD200 should (probably) not report a warning for DisposeAsyncCore method name as it is recommended here: https://docs.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-disposeasync#disposeasync-and-disposeasynccore

Repro steps

Code to reproduce the behavior.

/// <summary>
/// Frees managed resources used by the object.
/// </summary>
/// <returns>A <see cref="ValueTask">task</see> that represents the asynchronous dispose operation.</returns>
protected virtual async ValueTask DisposeAsyncCore() // <<<<<<< WARNING HERE
{
    this.log.Debug("*");

    lock (this.disposedValueLock)
    {
        if (this.disposedValue)
        {
            this.log.Debug("$<ALREADY_DISPOSED>");
            return;
        }

        this.disposedValue = true;
    }

    // ...

    this.log.Debug("$");
}

/// <inheritdoc/>
public async ValueTask DisposeAsync()
{
    await this.DisposeAsyncCore().ConfigureAwait(false);
    GC.SuppressFinalize(this);
}

Expected behavior

No warning.

Actual behavior

A warning is emitted.

@sharwell
Copy link
Member

It appears there is still an open discussion on this. Once that concludes I can update the analyzer with the result.

@Eli-Black-Work
Copy link

Eli-Black-Work commented Oct 17, 2022

@sharwell I just ran into this warning, too. Is the naming of DisposeAsyncCore() still up for discussion? 🙂

@MartyIX
Copy link
Contributor Author

MartyIX commented Oct 17, 2022

.NET SDK analyzers have the same issue: dotnet/roslyn-analyzers#3909 (AFAIK).

@Eli-Black-Work
Copy link

@MartyIX Thanks, I was curious if that was the case for the .NET SDK analyzers as well 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants