Skip to content

Suppress PREfast false positive (C28167) for RAII lock wrappers#629

Merged
jonwis merged 1 commit intomicrosoft:masterfrom
KintoCA:avarshavsky/fix-prefast-false-positives
Apr 7, 2026
Merged

Suppress PREfast false positive (C28167) for RAII lock wrappers#629
jonwis merged 1 commit intomicrosoft:masterfrom
KintoCA:avarshavsky/fix-prefast-false-positives

Conversation

@KintoCA
Copy link
Copy Markdown
Contributor

@KintoCA KintoCA commented Apr 2, 2026

PREfast warning C28167 ("The function changes the lock state and does not restore it") fires as a false positive on RAII-based locking patterns in resource.h. PREfast's interprocedural analysis cannot track that the lock is released inside a C++ destructor, causing it to incorrectly report that callers of WIL lock guards leave locks in an inconsistent state.

This change adds C28167 to the existing #pragma warning(disable ...) block that already suppresses the related C26135 and C26110 warnings for the same reason.

Testing:
Verified that the suppression eliminates the false-positive PREfast diagnostics without masking any real lock-state issues. No functional behavior changes.

@KintoCA KintoCA force-pushed the avarshavsky/fix-prefast-false-positives branch 4 times, most recently from 760e6d6 to a7d8620 Compare April 2, 2026 22:53
@KintoCA KintoCA force-pushed the avarshavsky/fix-prefast-false-positives branch from a7d8620 to 25ddb8e Compare April 2, 2026 22:55
@KintoCA
Copy link
Copy Markdown
Contributor Author

KintoCA commented Apr 3, 2026

CI is green and this PR is ready for review.

This change suppresses PREfast warning C28167 in resource.h, which appears to be a false positive for the RAII lock pattern because the analyzer does not correctly model lock release through C++ destructors. There is no intended functional or behavioral change.

@KintoCA
Copy link
Copy Markdown
Contributor Author

KintoCA commented Apr 6, 2026

@dunhor would you mind taking a look when you have a chance?

This suppresses PREfast C28167 for RAII lock wrappers, alongside the existing
C26135 / C26110 suppressions in the same block. Verified it removes the false
positive without masking real lock-state issues.

Happy to adjust if you’d prefer a different scope or placement.

@KintoCA
Copy link
Copy Markdown
Contributor Author

KintoCA commented Apr 7, 2026

@dunhor,
Thanks for the review!

Since I don’t have merge permissions on this repo, could someone please merge when convenient?

@jonwis jonwis merged commit 2ec8cf0 into microsoft:master Apr 7, 2026
15 checks passed
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.

3 participants