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

VM #define Sleep macros break GC #7959

Closed
sdmaclea opened this issue Apr 26, 2017 · 4 comments · Fixed by #70868
Closed

VM #define Sleep macros break GC #7959

sdmaclea opened this issue Apr 26, 2017 · 4 comments · Fixed by #70868
Assignees
Milestone

Comments

@sdmaclea
Copy link
Contributor

In various places, the VM introduces #define Sleep(a) Dont_Use_Sleep(a). These are troublesome for the GC which has to introduce a workaround.

Per @jkotas "this hack made sense for full .NET Framework. It is not really valuable for CoreCLR. I would rather delete all the code to #define Sleep to prevent people from using it."

My tip has these #define for Sleep in headers...

src/gc/gc.h:// This is a funny workaround for the fact that "common.h" defines Sleep to be
src/gc/gc.h:// However, GCToOSInterface defines a function called Sleep, which (due to this define) becomes
src/inc/clrhost.h:#define Sleep(dwMilliseconds) \
src/vm/comsynchronizable.h:#define Sleep(a) Dont_Use_Sleep(a)
src/vm/ecalllist.h:#define Sleep(a) Dont_Use_Sleep(a)
@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@mangod9
Copy link
Member

mangod9 commented Sep 22, 2020

@sdmaclea this appears to be fixed, or is this still a concern?

@sdmaclea
Copy link
Contributor Author

Looks like it is still a concern.

~/git/runtime$ git grep '#define *Sleep' origin/master
origin/master:src/coreclr/src/vm/comsynchronizable.cpp:#define Sleep(dwMilliseconds) Dont_Use_Sleep(dwMilliseconds)
origin/master:src/coreclr/src/vm/comsynchronizable.h:#define Sleep(a) Dont_Use_Sleep(a)
origin/master:src/coreclr/src/vm/ecalllist.h:#define Sleep(a) Dont_Use_Sleep(a)
origin/master:src/coreclr/src/vm/hosting.cpp:#define SleepEx(dwMilliseconds,bAlertable) \
origin/master:src/coreclr/src/vm/threads.cpp:#define Sleep(a) Dont_Use_Sleep(a)
origin/master:src/coreclr/src/vm/threadsuspend.cpp:#define Sleep(a) Dont_Use_Sleep(a)
origin/master:src/coreclr/src/vm/win32threadpool.cpp:#define SleepEx(a,b) Dont_Use_SleepEx(a,b)
origin/master:src/coreclr/src/vm/win32threadpool.cpp:                #define SleepEx(a,b) Dont_Use_SleepEx(a,b)
origin/master:src/coreclr/src/vm/win32threadpool.cpp:#define SleepEx(a,b) Dont_Use_SleepEx(a,b)

@mangod9
Copy link
Member

mangod9 commented Sep 22, 2020

I meant I didnt see the workaround you mention in gc.h. Perhaps it should be moved to vm area then?

@sdmaclea
Copy link
Contributor Author

Done

@mangod9 mangod9 modified the milestones: Future, 6.0.0 Sep 22, 2020
@mangod9 mangod9 modified the milestones: 6.0.0, 7.0.0 Jul 21, 2021
@AaronRobinsonMSFT AaronRobinsonMSFT self-assigned this Jun 17, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 17, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 17, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jul 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants