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

Allow AppDomain.SetData more than once #8029

Closed
bricelam opened this issue May 8, 2017 · 14 comments
Closed

Allow AppDomain.SetData more than once #8029

bricelam opened this issue May 8, 2017 · 14 comments

Comments

@bricelam
Copy link
Contributor

bricelam commented May 8, 2017

There seems to be an arbitrary limitation on Core CLR that prevents you from calling AppDomain.SetData more than once for the same name.

This limitation doesn't exist on .NET Framework.

We use AppDomain.SetData in our tests to check our usage of AppDomain.GetData. We typically clean up after the test by passing null, but this throws.

We discovered this by attempting to run our .NET Framework 4.6.1 test code on .NET Core App 2.0.

@gkhanna79
Copy link
Member

CC @danmosemsft

@danmoseley
Copy link
Member

@AlexGhiondea any context on why this code exists? It looks like it precedes github: https://referencesource.microsoft.com/#mscorlib/system/appdomain.cs,2444

Unless there's a good reason, we should fix this. If you don't have cycles, LMK and I'll find someone.

@danmoseley danmoseley assigned sepidehkh and unassigned AlexGhiondea May 9, 2017
@danmoseley
Copy link
Member

@sepidehms can you do this one for 5/10 ?

@sepidehkh
Copy link
Contributor

@danmosemsft I can't say for sure I will finish it for 5/10 with two other tasks I need to finish, but I certainly can spend time on this for investigation.

@danmoseley
Copy link
Member

OK!

@AlexGhiondea
Copy link
Contributor

@sepidehms the fix for this is to do something like what I have in this branch: https://github.com/AlexGhiondea/coreclr/tree/MakeSetData

@AlexGhiondea
Copy link
Contributor

I have a PR for this here: dotnet/coreclr#11496

@danmoseley
Copy link
Member

@ViktorHofer if @sepidehms doesn't have time to do this today could you please get the PR in.

@AlexGhiondea
Copy link
Contributor

@danmosemsft the PR is approved -- I am just waiting for CI to complete

@danmoseley
Copy link
Member

Great - thanks both of you.

@danmoseley
Copy link
Member

Fixed right @AlexGhiondea

@jkotas
Copy link
Member

jkotas commented May 11, 2017

We need regression test in corefx

@jkotas jkotas reopened this May 11, 2017
@karelz
Copy link
Member

karelz commented May 11, 2017

@AlexGhiondea should we move the test part into CoreFX? (we don't have to if it is easier this way)

@stephentoub stephentoub self-assigned this May 11, 2017
@stephentoub
Copy link
Member

I added a test here:
dotnet/corefx#19657

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the 2.0.0 milestone Jan 31, 2020
@dotnet dotnet locked as resolved and limited conversation to collaborators Dec 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants