Various fixes #390
Merged
Various fixes #390
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
leculver
commented
Dec 8, 2019
leculver
commented
- Make ClrHandle abstract. Since there are a lot of handles, use subclasses to limit the amount of memory we need to allocate.
- Stop using Debug.Assert. This causes all kinds of problems for our test cases.
- Removed SafeLoadLibraryHandle (dead code).
- Removed GetFieldForOffset. We don't implement this any better than anyone else can, and it could be replaced by a simple .FirstOrDefault expression.
- Renamed ClrType.BaseSize to StaticSize.
Removing dead code.
BaseSize is confusing as each type has a base type. Rename this to StaticSize, since this is actually the static size of the object created from this type.
Use subclasses to reduces individual handle size.
leculver
added a commit
that referenced
this pull request
Dec 9, 2019
* Clean up styles (#340) * Implement SuspendAndAttachToProcess on Linux (#337) * Implement SuspendAndAttachToProcess on Linux * Switch to ArrayPool.Rent * Remove legacy runtime support (#342) * Merge all ClrHeap subclasses - Merged all ClrHeap subclasses into one. * Remove legacy runtime support - Only support .Net Core and Desktop CLR 4.6+. * Fix bad exception in version check * Reimplement dbgeng interfaces (#343) * Initial version of DbgEng wrappers * Fix access violation * Fix build warning * Push IDebugClient out of ClrMD Move IDebugClient to the Util library. * Fix PEImage failure * Improve LinuxLiveDataReader (#345) * Improve LinuxLiveDataReader * Remove 'this' prefixes * Add test * Refactor codebase for 2.0 (#358) * First pass at decoupling runtime objects * WIP: Type implementation now compiles * WIP implemented Threads, Modules, AppDomains * WIP now ClrMD compiles * WIP tests compile except gcroot * WIP fix a couple test failures * WIP Fix module appdomain issue * Fix segment enumeration * WIP fix multiple test failures * Add ClrModule.GetTypeByName back * Fix shared/system domain names * Don't touch symbol server in tests Remove or change tests which touch the symbol server, as it greatly slows down execution. * Cleanup domain init * Fix memory corruption in LiveDataReader Need to port to 1.0. * Implement GetOrCreateBasicType * Fix allocation context problem Inverted up HashSet.Add meaning. * Remove heap/runtime paramters from ITypeFactory * ClrThread.Runtime was not set * Fix ClrModule.AppDomain == null * Remove StaticVarRoot class * Fix incorrect field properties * Fix primitive type ElementType * Fix ClrType.ClrObjectHelpers * Fix read error Inverted if meant we failed to assign memory. * Remove ClrObject DebuggerDisplay * Fix SystemDomain issue * Clean up GetTypeByName * Use more specific field element type * Choose whether to include thread context * Implement ClrmdStackFrame.ToString * Fix ClrmdMethod.MethodDesc * Fix ClrMethod enumeration * Clean up CreateMethodsForType allocation * Disable symbol server in tests * Reuse ClrMethod and don't recreate them * Fix ClrObject.GetStringField * Fix fields in types with no fields * Fix attach issue * Fix null-ref * Fix incorrect field length issue Need to test against 1.1. * Implement GetOrCreateTypeFromToken * Test fixes * Fix PdbInfo * Remove ContextHelper * Fix Finalizer code * Refactor how GC roots work * Get GCRoot compiling, rename Util - GCRoot now compiles, renamed the utilities project. * Update test to reflect reality * Clean up public facing types * Begin .Desktop cleanup * Rename Desktop as Implementation * Clean up Implementation folder * Remove ClrThreadPool This doesn't work in recent versions of the runtime. Need to rethink that. * More Implementation cleanup * Fix build warnings * Clear out failing tests * Turn off parallel * Add DependentHandle support * Fix non-deterministic type test failure * Remove failing tests * Clean up build warnings * Change how we find the BCL * Pull basic types from BCL * Clean up various classes Clean up PdbInfo, ModuleInfo, DacInfo, etc. * Rework ClrStaticField * Target 4.7.1 instead of 4.7.2. (#361) * Don't read from /proc/<pid>/mem (#351) * Don't read from /proc/<pid>/mem * Throw UnauthorizedAccessException for permission errors * Don't set bytesRead to a negative value * Throw InvalidOperationException if the process has exited * Fix permission parser (#357) * Rework instance field GetValue method (#362) * ComponentTypeEventuallyFilledTest (#363) Added test to ensure that issue #108 is fixed in 2.0 and isn't regressed. * Fix struct field size (#364) * Add (skipped) test for String.Empty issue (#366) We cannot get static values out of the BCL currently due to this issue. It seems to be a problem in the dac, not in ClrMD. * Fields and segments (#367) * Fix missing segments Segment enumeration was mistakenly started from the EphemeralHeapSegment instead of the first Gen2 segment. * Fix field issue - We did not properly account for the meaaning of fieldInfo.NumFields. This includes the base type's field count, so we did not need to add that. - ContextStatics and ThreadStatics are included in "NumStaticFields". Need to be sure count those as we are walking fields. * Clean up memory usage (#368) Reduce some obvious memory usage. This is a precursor to adding a mechanism to tell ClrMD what to cache. For example, don't method names, or ClrType instances, etc. That's a larger change that will require implementing equality comparison across ClrType/ClrMethod/Clr*Field Checkins: * Reduce ClrmdType memory usage * Remove nullable * Split RuntimeBuilder into mutliple classes * Cleanups * Remove TypeCache * Fix NRE in PEImage.IsManaged (#370) * Fix NRE in PEImage.IsManaged * Update comment * Apply suggestion from #326 * Implement GetFileVersion/GetVersionInfo on Linux (#349) * Add max length of string reads (#371) Added a maximum length to read from strings to prevent OOM/Overflow if there's corrupt memory. Fixes #244. * Ensure ClrRuntime properly cleans up (#372) -ClrRuntime is now an IDisposable. -Properly disposing of ClrRuntime ensures that all interfaces and the DacLibrary are completely released. Not disposing of ClrRuntime is fine, but cleanup and release of libraries and interfaces will happen on the finalizer thread. -ClrRuntime.FlushCachedData is now properly implemented and mostly tested. * Implement module and type equality (#373) * Implement type equality * Rename TypeHandle back to MethodTable It'll just be too confusing. * Add module equality * Use is null instead of == null Fix stack overflow caused by recursive operator ==. * Clean up styles (#356) * Use string.Empty * null coalescing * Roslyn cleanup * Add license banners * Add modifiers * Remove empty comments * Remove spaces * Remove blank lines * Add blank lines * Add periods * Use braces consistently * Constants on the right * Allow ClrMD to be used from multiple threads (#376) Initial implementation of support for parallel processing. Not all data targets have been marked as threadsafe yet. Only DbgEngDataReader and Windows's LiveDataReader are currently thread safe. Note that we do not expect to see any perf improvements by running ClrMD operations in parallel. This is because the underlying diagnostics API mscordac*.dll takes a giant lock around any call we make, and many IDataTarget implementations will similarly need to take a lock around from the target. Using ClrMD from multiple threads is primarily for convenience. That is, not having to keep ClrMD locked to a single thread, you can now use it with in an async/await context, for example. This also allows parallel processing for user code if you are in a scenario where you only need to read from the dac/data target a little and do a lot of processing yourself. Changes: * Ensure we only ever produce a single heap * Allow recreating ClrRuntime * Make MemoryReader a thread static * Add parallel stress test * Add .xml to the ignore list to stop checking the files in * Clean up ParallelStressTest * Inline size calculation * Add heap walk logging * Rename IsMinidump * Add IDataReader.IsThreadSafe * Add ClrRuntime/ITypeFactory.IsThreadSafe * Lock cache before clearing it * Clean up ClrmdHeap data reading (#377) - Wrap heap logging into its own function. - Stop using MemoryReader for the large object heap. Objects on the LoH are 85,000 bytes or more, so we don't need to read an entire chunk of memory just for the MethodTable and array count. * Annotate for nullable reference types (#379) * Annotate Microsoft.Diagnostics.Runtime for nullable reference types * Fix all outstanding build warnings * Silence build warning * Implement caching options (#383) * Implement cached options for ClrType * Implement Method and MethodName caching * Implement type and type name caching * Implement field and field name caching * Split tests * Minor cleanups (#384) * Don't null check when we know the result is non-null * ClrObject: Change back to NullReferenceException * Report the object reference even when enumerating the stack even if we can't get the type * Allow FlushCachedData to be called while enumerating the heap (#385) * Fix unsynchronized IXClrDataProcess::Flush corruption (#386) IXClrDataProcess::Flush is unfortunately not wrapped with DAC_ENTER.This means that when it starts deleting memory, it's completely unsynchronized with parallel reads and writes, leading to heap corruption and other issues. This means that in order to properly clear dac data structures, we need to trick the dac into entering the criticalse ction for us so we can call Flush safely then. To accomplish this, we set a hook in our implementation of DacDataTarget::ReadVirtual which will call IXClrDataProcess::Flush if the dac tries to read the address set by MagicCallbackConstant.Additionally we make sure this doesn't interfere with other reads by 1) Ensuring that the address is in kernel space, 2) only calling when we've entered a special context. * Fix stale finalizer queue objects * Fix stale dependent handles * Clean up heap initialization * Stop using ConcurrentBag (#389) Thread static fields keep ConcurrentBag alive too long. This lead to fast growing memory leaks. We don't need something as heavyweight as ConcurrentBag. A queue with a lock is just fine. * Various fixes (#390) * Delete SafeLoadLibraryHandle.cs Removing dead code. * Remove GetFieldForOffset * Rename BaseSize BaseSize is confusing as each type has a base type. Rename this to StaticSize, since this is actually the static size of the object created from this type. * Clean up ClrHandle implementation Use subclasses to reduces individual handle size. * Stop using Debug.Assert * Fix dependent handle issue * Rewrite SymbolLocator (#392) - Remove SymbolLocator/DefaultSymbol locator. ClrMD's symbol server implementation was buggy and incomplete. If it's needed in the future people should use the symbol server client provided provided by the CLR Diagnostics team. - Added a new (much simpler) class that communicates with the symbol server to implement the same basic functionality. - Added back symbol tests, but by default they are skipped. * Create a new BinaryLocator * Fix Release build issue * Update Linux symbol locator * Rename BinaryLocator * Add back SymbolLocatorTests Added back SymbolLocatorTests, but skipped. * Rename tests * Remove SymbolLocator code * Fix symbol path * Put GCRoot back into ClrMD (#395) Moving GCRoot class back into ClrMD now that I feel much better about the implementation. - Greatly simplified the logic of the class. - Use threads instead of tasks for processing. In most non-trivial cases of GCRoot the amount of work that has to be done is staggering. Creating tasks is a huge amount of overhead for something that could be a simple producer/consumer. - Fix flaky GCRoot tests. The test that would randomly fail is due to non-deterministic ordering of which roots are returned during a 'unique' root walk. - Fix synchronization across worker threads. Before, objects would be added to the 'seen' set during processing, even if the object ended up being a real root. This caused non-unique root walks to return non-deterministic numbers of roots. * Simplify GCRoot * Fix GCRoot synchronization issue We would mark an object as 'seen' while we are still processing it. This would lead to the scenario where one processing thread would skip a rooting chain because it was being processed but hadn't reached the target object. * Move GCRoot back into ClrMD Feeling better about the code, putting back into the library. * Send GCRoot progress update events * Throw PlatformNotSupportedException in DataTarget (#393) * Fix tests * Stop using Nullable<T>.Value (#396) * Update FixGenerics to work when there's no arity (#397) Fixes #394. Ensure we correctly parse generic type names without the grave airty. * Add FileVersionInfo to PEImage (#399) Fixes #398. * Update FXCop build * Update version to 2.0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.