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

Revert workarounds for mono/linker#2181 #57451

Merged
merged 2 commits into from
Aug 20, 2021

Conversation

vitek-karas
Copy link
Member

@vitek-karas vitek-karas commented Aug 15, 2021

This reverts the ArrayPool workaround for mono/linker@2181 as that issue was fully fixed and the fix is in the runtime repo now.

@vitek-karas vitek-karas added the linkable-framework Issues associated with delivering a linker friendly framework label Aug 15, 2021
@vitek-karas vitek-karas self-assigned this Aug 15, 2021
@ghost
Copy link

ghost commented Aug 15, 2021

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr
See info in area-owners.md if you want to be subscribed.

Issue Details

Trying to revert both.
The Http3RequestStream is definitely fixed by the linker changes.
The ArrayPool ones probably not, but trying to get a repro.

Author: vitek-karas
Assignees: vitek-karas
Labels:

linkable-framework

Milestone: -

@lewing
Copy link
Member

lewing commented Aug 16, 2021

runtime (Libraries Test Run checked coreclr windows x86 Release) was #57452

@vitek-karas
Copy link
Member Author

@stephentoub @vargaz - I can't seem to repro the failure from #56316 (comment). Do you remember if there was something special needed? Or do I need to revert more changes?

I honestly don't think that the fix made in dotnet/linker#2205 would fix problems with invalid finally (the fix is solely around isinst instruction which doesn't occur in the IL reported in #56316 (comment).

@steveisok Could you please review the change in Http3RequestStream? Specifically, if it's all of the workaround being removed.

@vitek-karas vitek-karas marked this pull request as ready for review August 16, 2021 12:45
@stephentoub
Copy link
Member

There was nothing special needed. However... in order to apply the ifdefs with minimal ceremony, I did invert one or two of the if/else blocks:
4ea83d2
and I don't think I tried doing that swap without the ifdefs. Maybe see if reverting that commit brings back the issue?

Trying to revert both.
The Http3RequestStream is definitely fixed by the linker changes.
The ArrayPool ones probably not, but trying to get a repro.
@vitek-karas
Copy link
Member Author

@stephentoub could you please take another look - this should be ready. Thanks a lot!

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

Thanks!

@stephentoub stephentoub merged commit f53c8dc into dotnet:main Aug 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 19, 2021
@vitek-karas vitek-karas deleted the RevertWorkaround branch September 26, 2023 09:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Http linkable-framework Issues associated with delivering a linker friendly framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants