Skip to content

Ensure all Dispose() implementations are idempotent and thread safe #946

Description

@jpdillingham

While working on the Listener and ensuring that it cleans itself up if there's an error, I went down a bit of a rabbit hole with hypotheticals and arrived at the realization that:

  • I need to implement a TryDispose() extension that swallows exceptions, for cases where a Dispose() needs to Dispose multiple other things; if one throws the others aren't disposed
  • I should (but don't have to if I use TryDispose() everywhere) go through all of my Dispose() implementations and ensure they are both idempotent and thread safe

Idempotentcy should be attempted in every implementation via a Disposed boolean that's set in the body of the Dispose() that does the work, but if any of those calls throw, it's not set, and if another thread enters the critical section before Disposed is set, everything will get called twice.

The impact of this, if it really is a problem, is at worst a resource leak, at next-worst stuff avoiding GC longer than it should, and at best not happening.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    refactorInternal refactoring/code improvements

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions