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

BaseBucketParamsManager::GetAppVersion should switch to PREEMPT before calling DwGetFileVersionInfo #35866

Closed
Maoni0 opened this issue May 5, 2020 · 6 comments
Assignees
Milestone

Comments

@Maoni0
Copy link
Member

Maoni0 commented May 5, 2020

I'm observing this code from EH is preventing suspension from completing in a timely manner -

+ coreclr!IL_Throw
+ coreclr!RaiseTheExceptionInternalOnly
+ kernelbase!RaiseException
+ ntdll!RtlRaiseException
+ ntdll!RtlDispatchException
+ ntdll!RtlpExecuteHandlerForException
+ coreclr!__C_specific_handler
+ coreclr!CallDescrWorkerReflectionWrapper'::1'::filt$0
+ coreclr!ReflectionInvocationExceptionFilter
+ coreclr!SetupWatsonBucketsForNonPreallocatedExceptions
+ coreclr!GetBucketParametersForManagedException
+ coreclr!GetManagedBucketParametersForIp
+ coreclr!CLR20r3BucketParamsManager::PopulateBucketParameters
+ coreclr!BaseBucketParamsManager::GetAppVersion
+ coreclr!DwGetFileVersionInfo
+ coreclr!GetFileVersion
+ coreclr!GetFileVersionInfoSizeW_NoThrow
+ kernelbase!GetFileVersionInfoSizeExW
+ kernelbase!LoadLibraryExW
+ kernelbase!BasepLoadLibraryAsDataFileInternal
+ ntdll!RtlDosSearchPath_Ustr
+ ntdll!RtlDoesFileExists_UstrEx
+ ntdll!ZwQueryAttributesFile
+ ntoskrnl!NtQueryAttributesFile
+ ntoskrnl!ObOpenObjectByNameEx
+ ntoskrnl!ObpLookupObjectName
+ ntoskrnl!IopParseDevice
+ ntoskrnl!IopQueryInformation
+ fltmgr!FltpFastIoQueryOpen
+ fltmgr!FltpPerformFastIoCall
+ ntfs!NtfsNetworkOpenCreate
+ ntfs!NtfsCommonQueryOpen
+ ntfs!NtfsCommonCreate
+ ntfs!NtfsAcquireSharedVcb
+ ntoskrnl!ExAcquireResourceSharedLite
+ ntoskrnl!ExpAcquireResourceSharedLite
+ ntoskrnl!ExpWaitForResource
+ ntoskrnl!KeWaitForSingleObject
+ ntoskrnl!KiCommitThreadWait
+ ntoskrnl!KiSwapThread
+ ntoskrnl!KiDeliverApc
+ ntoskrnl!PspGetSetContextSpecialApc
+ ntoskrnl!KeSignalGate
+ ntoskrnl!KiExitDispatcher
+ EventData Readied Thread 7736 Proc 21104
+ Event Windows Kernel/Dispatcher/ReadyThread

or DwGetFileVersionInfo could do that before calling GetFileVersion. this is causing suspension that's 3 to 4s long.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label May 5, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this issue. Please help me learn by adding exactly one area label.

@Maoni0 Maoni0 added area-ExceptionHandling-coreclr and removed untriaged New issue has not been triaged by the area owner labels May 5, 2020
@jkotas jkotas added the tenet-performance Performance related issue label May 6, 2020
@jkotas jkotas added this to the 5.0 milestone May 6, 2020
@mangod9 mangod9 self-assigned this May 6, 2020
@mangod9
Copy link
Member

mangod9 commented May 8, 2020

@Maoni0 I have a presumptive fix here: #36151. Though from what I can see the watson codepath should only be hit in unhandled exception scenarios?

@Maoni0
Copy link
Member Author

Maoni0 commented May 8, 2020

chatted with some folks who are more familiar with EH about this. the gist is there may be code paths that handle exceptions that the runtime may not know for sure are unhandled (so they may or may not be unhandled later), at least this was the case on desktop. also this does hit a particular EH which is ReflectionInvocationExceptionFilter. the process clearly kept on running for the case I looked at.

@mangod9
Copy link
Member

mangod9 commented May 9, 2020

@mangod9
Copy link
Member

mangod9 commented May 9, 2020

Ok I was able to hit the watson codepath with the reflection invocation exception case. Looks like ReflectionInvocationExceptionFilter switches to COOP just before calling SetupWatsonBucketsForNonPreallocatedExceptions:

Assume that is required to gcprotect oThrowable?

@mangod9
Copy link
Member

mangod9 commented May 11, 2020

fix has been merged

@mangod9 mangod9 closed this as completed May 11, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 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

4 participants