-
Notifications
You must be signed in to change notification settings - Fork 214
Add System.Private.DeveloperExperience.Console #71
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
Add System.Private.DeveloperExperience.Console #71
Conversation
Since this System.Private library depends on Console (and exists solely because CoreLib cannot reference System.Console), placing this in the libs partition. I'm not actually porting all of System.Private.DeveloperExperience.Console - the dependency on System.Private.StackTraceGenerator was annoying so I removed it. I'm deleting System.Private.StackTraceGenerator - this only works on Windows systems that have DIA registered machine wide - that's not the case (by default) since VS 2015.
...em.Private.DeveloperExperience.Console/src/System.Private.DeveloperExperience.Console.csproj
Outdated
Show resolved
Hide resolved
...em.Private.DeveloperExperience.Console/src/System.Private.DeveloperExperience.Console.csproj
Outdated
Show resolved
Hide resolved
jkotas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
src/coreclr/src/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets
Show resolved
Hide resolved
...src/nativeaot/System.Private.CoreLib/src/Internal/DeveloperExperience/DeveloperExperience.cs
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| public virtual string CreateStackTraceString(IntPtr ip, bool includeFileInfo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not need to be public with virtual methods anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does if we want to bring back dotnet/corert#7658 at some point.
jkotas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM otherwise
...src/nativeaot/System.Private.CoreLib/src/Internal/DeveloperExperience/DeveloperExperience.cs
Outdated
Show resolved
Hide resolved
This change rotates the encryption key used for protecting Stateless Retry tokens every 30 seconds. The rotation is performed in a lazy manner, to avoid needing a timer, and only actually occurs when a Stateless Retry token is being sent from the server. In order to prevent the encryption key from being freed while in use, all callers are required to hold a global lock while using the key. This could impact performance and can be improved by either ref-counting the keys, or by having encryption keys duplicated per CPU. The existing tests already cover Stateless Retry tokens being sent. A test case is needed for the negative case where the client responds with the token after the key for that token has been rotated.
Since this System.Private library depends on Console (and exists solely because CoreLib cannot reference System.Console), placing this in the libs partition.
I'm not actually porting all of System.Private.DeveloperExperience.Console - the dependency on System.Private.StackTraceGenerator was annoying so I removed it. I'm deleting System.Private.StackTraceGenerator - this only works on Windows systems that have DIA registered machine wide - that's not the case (by default) since VS 2017.
Depends on #70.