Skip to content

Conversation

@VSadov
Copy link
Member

@VSadov VSadov commented Aug 16, 2021

Fixes:#57137

This is just a sanity assert to make sure we use the InitializeContext API correctly, which we do.
However the assert seems to be triggered in some Win7 runs with no impact on actual functionality. The easiest way to deal with that is just to disable the assert.

@ghost ghost added the area-GC-coreclr label Aug 16, 2021
@ghost
Copy link

ghost commented Aug 16, 2021

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes:#57137

This is just a sanity assert to make sure we use the InitializeContext API correctly, which we do.
However the assert seems to be triggered in some Win7 runs with no impact on actual functionality. The easiest way to deal with that is just to disable the assert.

Author: VSadov
Assignees: -
Labels:

area-GC-coreclr

Milestone: -

@VSadov
Copy link
Member Author

VSadov commented Aug 17, 2021

Thanks!!

@ghost
Copy link

ghost commented Aug 17, 2021

Hello @VSadov!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we know what condition fails on win7. Assume that the error returned is not ERROR_INSUFFICIENT_BUFFER?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it cannot succeed with NULL buffer, so most likely it is a different or lost error code.
I do not have Win7 available and as I understand it does not always cause the assert. It is curious, but could be time consuming to find out.

In any case, if this API is not telling us the desired size of the buffer it would be a situation that we cannot handle and it would be fatal in the next call to this API anyways. It would also indicate that something went really wrong on the OS side.

I do not think the value of this assert is very high. I think we use the API correctly and with the comments around it is unlikely to accidentally regress.

@jkoritzinsky
Copy link
Member

The OSX changes are a Helix infra issue. The Windows x86 test failure is also intermittently failing in main.

@VSadov I think we can merge this if you want to.

@VSadov
Copy link
Member Author

VSadov commented Aug 17, 2021

@jkoritzinsky - Thanks! I prefer PRs to merge green, but I do not see how these failures are even remotely related to removal of an assert. I will merge this.

@VSadov VSadov merged commit f7cd0c6 into dotnet:main Aug 17, 2021
@VSadov VSadov deleted the win7assert branch August 17, 2021 17:21
@ghost ghost locked as resolved and limited conversation to collaborators Sep 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants