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

Inherit thread affinity from base types and members #231

Open
sharwell opened this issue Apr 3, 2018 · 5 comments
Open

Inherit thread affinity from base types and members #231

sharwell opened this issue Apr 3, 2018 · 5 comments

Comments

@sharwell
Copy link
Member

sharwell commented Apr 3, 2018

  • Types derived from a type that requires the main thread should also have a main thread requirement
  • Members derived from a member that requires the main thread should also have a main thread requirement

Then as a follow-up to the above:

  • Members which have a main thread requirement due to inheritance should not report a warning if they are missing a ThrowIfNotOnUIThread call
@AArnott
Copy link
Member

AArnott commented Apr 4, 2018

Members derived from a member that requires the main thread should also have a main thread requirement

By a derived member are you talking about an override member?

Members which have a main thread requirement due to inheritance should not report a warning if they are missing a ThrowIfNotOnUIThread call

I disagree. There is no guarantee that the caller has the threading analyzers applied to their project with the right analyzer turned to warning/error. At runtime something that requires the UI thread should still throw if it's not called on the UI thread in order to prefer throwing exceptions over deadlocking.

@sharwell
Copy link
Member Author

sharwell commented Apr 4, 2018

By a derived member are you talking about an override member?

  • override: Yes
  • Explicit interface implementation: Yes
  • Implicit interface implementation: Probably, but could be persuaded otherwise and may not be necessary. Biggest advantage would be code implementing an interface provided by some service, where the service only ever calls into the interface from the UI thread. By marking the interface and having it propagate to implementations, the analyzer could assume the entry point to each method in the implementation is on the UI thread.

@sharwell
Copy link
Member Author

sharwell commented Apr 4, 2018

I disagree. There is no guarantee that the caller has the threading analyzers applied to their project with the right analyzer turned to warning/error. At runtime something that requires the UI thread should still throw if it's not called on the UI thread in order to prefer throwing exceptions over deadlocking.

This is fine, but if the behavior is kept it should use a different diagnostic ID. Otherwise it's an overwhelming source of noise for code that defines hundreds or thousands of event handlers for UI events.

@AArnott
Copy link
Member

AArnott commented Apr 4, 2018

Event handlers I don't mind removing the requirement from under the conditions you proposed. It's members that are accessible in other ways that I think should still defend themselves from improper calling.

@sharwell
Copy link
Member Author

sharwell commented Apr 4, 2018

That seems more relevant for a library or application with a public API. For a contained application, it's just noise (all the entry points to affinitized code would be analyzed separately).

AArnott pushed a commit to AArnott/vs-threading that referenced this issue Jan 5, 2024
Bumps [powershell](https://github.com/PowerShell/PowerShell) from 7.3.9 to 7.4.0.
- [Release notes](https://github.com/PowerShell/PowerShell/releases)
- [Commits](PowerShell/PowerShell@v7.3.9...v7.4.0)

---
updated-dependencies:
- dependency-name: powershell
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants