From ec4683278c45479504bdfecf32769e0413a82d98 Mon Sep 17 00:00:00 2001 From: sawilde Date: Sun, 27 Sep 2015 12:28:51 +1000 Subject: [PATCH 01/10] prep for talk --- main/OpenCover.Framework/Manager/ProfilerManager.cs | 2 ++ .../Framework/Manager/ProfilerManagerTests.cs | 8 ++++---- main/cmdline/dogfood.cmd | 3 +++ 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/main/OpenCover.Framework/Manager/ProfilerManager.cs b/main/OpenCover.Framework/Manager/ProfilerManager.cs index 545776677..e8a8e5d72 100644 --- a/main/OpenCover.Framework/Manager/ProfilerManager.cs +++ b/main/OpenCover.Framework/Manager/ProfilerManager.cs @@ -127,6 +127,8 @@ private void SetProfilerAttributesOnDictionary(string profilerKey, string profil dictionary["Cor_Enable_Profiling"] = "1"; dictionary["CoreClr_Profiler"] = ProfilerGuid; dictionary["CoreClr_Enable_Profiling"] = "1"; + dictionary["Cor_Profiler_Path"] = string.Empty; + dictionary["CorClr_Profiler_Path"] = string.Empty; switch (_commandLine.Registration) { diff --git a/main/OpenCover.Test/Framework/Manager/ProfilerManagerTests.cs b/main/OpenCover.Test/Framework/Manager/ProfilerManagerTests.cs index 999053948..e7c593333 100644 --- a/main/OpenCover.Test/Framework/Manager/ProfilerManagerTests.cs +++ b/main/OpenCover.Test/Framework/Manager/ProfilerManagerTests.cs @@ -146,8 +146,8 @@ public void Manager_DoesNotAdd_Cor_Profiler_Path_EnvironmentVariable_WithNormalR RunSimpleProcess(dict); // assert - Assert.IsFalse(dict.ContainsKey(@"Cor_Profiler_Path")); - Assert.IsFalse(dict.ContainsKey(@"CorClr_Profiler_Path")); + Assert.IsFalse(!(dict.ContainsKey(@"Cor_Profiler_Path") || !string.IsNullOrEmpty(dict[@"Cor_Profiler_Path"]))); + Assert.IsFalse(!(dict.ContainsKey(@"CorClr_Profiler_Path") || !string.IsNullOrEmpty(dict[@"CorClr_Profiler_Path"]))); } [Test] @@ -161,8 +161,8 @@ public void Manager_DoesNotAdd_Cor_Profiler_Path_EnvironmentVariable_WithUserReg RunSimpleProcess(dict); // assert - Assert.IsFalse(dict.ContainsKey(@"Cor_Profiler_Path")); - Assert.IsFalse(dict.ContainsKey(@"CorClr_Profiler_Path")); + Assert.IsFalse(!(dict.ContainsKey(@"Cor_Profiler_Path") || !string.IsNullOrEmpty(dict[@"Cor_Profiler_Path"]))); + Assert.IsFalse(!(dict.ContainsKey(@"CorClr_Profiler_Path") || !string.IsNullOrEmpty(dict[@"CorClr_Profiler_Path"]))); } [Test] diff --git a/main/cmdline/dogfood.cmd b/main/cmdline/dogfood.cmd index 7d00a3c70..17ec381f5 100644 --- a/main/cmdline/dogfood.cmd +++ b/main/cmdline/dogfood.cmd @@ -1 +1,4 @@ +pushd %cd% +cd %~dp0 OpenCover.Console.exe -register:user -target:..\..\..\main\packages\NUnit.Runners.2.6.4\tools\nunit-console-x86.exe -targetargs:"OpenCover.Test.dll /noshadow /exclude:AdminOnly" -filter:"+[Open*]* -[OpenCover.T*]* -{nunit-console*}[*]* -{pdb*}[*]*" -output:opencovertests.xml +popd \ No newline at end of file From 17983aea46085bf2d498de5fec0b06246bf072d5 Mon Sep 17 00:00:00 2001 From: sawilde Date: Sun, 8 Nov 2015 15:59:35 +1100 Subject: [PATCH 02/10] housekeeping --- main/OpenCover.Console/Program.cs | 4 +- main/OpenCover.Framework/Bootstrapper.cs | 4 + .../Communication/CommunicationManager.cs | 15 +++- .../Communication/MarshalWapper.cs | 13 ++++ .../Communication/MessageHandler.cs | 14 ++++ .../Communication/Messages.cs | 10 ++- main/OpenCover.Framework/Filter.cs | 73 ++++++++++++++++++- .../Manager/MemoryManager.cs | 10 ++- .../Manager/ProfilerManager.cs | 27 ++++++- main/OpenCover.Framework/Model/Class.cs | 4 + main/OpenCover.Framework/Model/File.cs | 4 +- .../InstrumentationModelBuilderFactory.cs | 6 ++ main/OpenCover.Framework/Model/Method.cs | 6 +- main/OpenCover.Framework/Model/Module.cs | 8 +- .../Model/SequencePoint.cs | 6 ++ .../Model/SkippedEntity.cs | 8 +- .../Model/SummarySkippedEntity.cs | 3 + .../Model/TrackedMethod.cs | 4 +- .../Persistance/BasePersistance.cs | 3 + .../Persistance/FilePersistance.cs | 3 + .../Strategy/TrackedMethodStrategyManager.cs | 10 +++ .../Utility/PerfCounters.cs | 23 +++++- 22 files changed, 232 insertions(+), 26 deletions(-) diff --git a/main/OpenCover.Console/Program.cs b/main/OpenCover.Console/Program.cs index e87483d16..1a23ecfac 100644 --- a/main/OpenCover.Console/Program.cs +++ b/main/OpenCover.Console/Program.cs @@ -109,7 +109,7 @@ private static void ReportCrash(Exception exception) uploader.SendAnonymousReport(SendRequestState.GetClientLib(), state.GetApplication(), state.GetExceptionDescription(true)); } - catch (Exception ex) + catch (Exception) { System.Console.WriteLine("Failed to send crash report :("); } @@ -303,7 +303,7 @@ private static void RunService(CommandLineParser parser, Action + /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources. + /// + /// 2 public void Dispose() { if (_container == null) return; diff --git a/main/OpenCover.Framework/Communication/CommunicationManager.cs b/main/OpenCover.Framework/Communication/CommunicationManager.cs index c737ae9e7..b2b1dce5b 100644 --- a/main/OpenCover.Framework/Communication/CommunicationManager.cs +++ b/main/OpenCover.Framework/Communication/CommunicationManager.cs @@ -1,10 +1,7 @@ using System; -using System.Diagnostics; using System.IO; using System.Threading; -using log4net.Repository.Hierarchy; using OpenCover.Framework.Manager; -using OpenCover.Framework.Service; namespace OpenCover.Framework.Communication { @@ -48,6 +45,11 @@ public CommunicationManager(IMessageHandler messageHandler) _messageHandler = messageHandler; } + /// + /// Process a communication related message from a profiler + /// + /// + /// public void HandleCommunicationBlock(IManagedCommunicationBlock mcb, Action offloadHandling) { mcb.ProfilerRequestsInformation.Reset(); @@ -71,6 +73,10 @@ private static void SendChunkAndWaitForConfirmation(int writeSize, IManagedCommu mcb.InformationReadByProfiler.Reset(); } + /// + /// process a results block from the profiler + /// + /// public byte[] HandleMemoryBlock(IManagedMemoryBlock mmb) { mmb.ProfilerHasResults.Reset(); @@ -88,6 +94,9 @@ public byte[] HandleMemoryBlock(IManagedMemoryBlock mmb) return newData; } + /// + /// Communication is over + /// public void Complete() { _messageHandler.Complete(); diff --git a/main/OpenCover.Framework/Communication/MarshalWapper.cs b/main/OpenCover.Framework/Communication/MarshalWapper.cs index 1f4f71d33..1531b7000 100644 --- a/main/OpenCover.Framework/Communication/MarshalWapper.cs +++ b/main/OpenCover.Framework/Communication/MarshalWapper.cs @@ -39,11 +39,24 @@ public interface IMarshalWrapper /// public class MarshalWrapper : IMarshalWrapper { + /// + /// Map pinned memory to a structure + /// + /// The type of the structure + /// + /// public T PtrToStructure(IntPtr pinnedMemory) { return (T)Marshal.PtrToStructure(pinnedMemory, typeof(T)); } + /// + /// Map a structure to pinned memory + /// + /// + /// + /// + /// public void StructureToPtr(T structure, IntPtr pinnedMemory, bool fDeleteOld) { Marshal.StructureToPtr(structure, pinnedMemory, fDeleteOld); diff --git a/main/OpenCover.Framework/Communication/MessageHandler.cs b/main/OpenCover.Framework/Communication/MessageHandler.cs index 99f246257..d49a1152b 100644 --- a/main/OpenCover.Framework/Communication/MessageHandler.cs +++ b/main/OpenCover.Framework/Communication/MessageHandler.cs @@ -67,6 +67,14 @@ public MessageHandler(IProfilerCommunication profilerCommunication, IMarshalWrap _memoryManager = memoryManager; } + /// + /// Process a Standard Message + /// + /// + /// + /// + /// + /// public int StandardMessage(MSG_Type msgType, IManagedCommunicationBlock mcb, Action chunkReady, Action offloadHandling) { IntPtr pinnedMemory = mcb.PinnedDataCommunication.AddrOfPinnedObject(); @@ -319,6 +327,9 @@ private int HandleTrackProcessMessage(IntPtr pinnedMemory) private int _readSize; + /// + /// Maximum size of a base message + /// public int ReadSize { get @@ -344,6 +355,9 @@ public int ReadSize } } + /// + /// Finished + /// public void Complete() { _profilerCommunication.Stopping(); diff --git a/main/OpenCover.Framework/Communication/Messages.cs b/main/OpenCover.Framework/Communication/Messages.cs index ba1e40846..49c423669 100644 --- a/main/OpenCover.Framework/Communication/Messages.cs +++ b/main/OpenCover.Framework/Communication/Messages.cs @@ -10,7 +10,8 @@ namespace OpenCover.Framework.Communication /// /// The command supportd by the host /// -// ReSharper disable InconsistentNaming + // ReSharper disable InconsistentNaming + // ReSharper disable once EnumUnderlyingTypeIsInt public enum MSG_Type : int { /// @@ -338,8 +339,15 @@ public struct MSG_AllocateBuffer_Request [StructLayout(LayoutKind.Sequential, Pack = 1)] public struct MSG_AllocateBuffer_Response { + /// + /// is the buffer allocated + /// [MarshalAs(UnmanagedType.Bool)] public bool allocated; + + /// + /// The id assigned to the buffer + /// public uint bufferId; } diff --git a/main/OpenCover.Framework/Filter.cs b/main/OpenCover.Framework/Filter.cs index 1501cb276..5211ddd69 100644 --- a/main/OpenCover.Framework/Filter.cs +++ b/main/OpenCover.Framework/Filter.cs @@ -5,7 +5,6 @@ // using System; using System.Collections.Generic; -using System.Diagnostics; using System.IO; using System.Linq; using System.Runtime.CompilerServices; @@ -25,6 +24,10 @@ public class Filter : IFilter internal IList ExcludedAttributes { get; private set; } internal IList ExcludedFiles { get; private set; } internal IList TestFiles { get; private set; } + + /// + /// Are the filters supplied as reguar expressions + /// public bool RegExFilters { get; private set; } @@ -42,6 +45,13 @@ public Filter(bool useRegexFilters = false) RegExFilters = useRegexFilters; } + /// + /// Decides whether an assembly should be included in the instrumentation + /// + /// The name of the process being profiled + /// the name of the assembly under profile + /// All assemblies matching either the inclusion or exclusion filter should be included + /// as it is the class that is being filtered within these unless the class filter is * public bool UseAssembly(string processName, string assemblyName) { processName = Path.GetFileNameWithoutExtension(processName); @@ -65,6 +75,13 @@ public bool UseAssembly(string processName, string assemblyName) return false; } + /// + /// Determine if an [assemblyname]classname pair matches the current Exclusion or Inclusion filters + /// + /// The name of the process + /// the name of the assembly under profile + /// the name of the class under profile + /// false - if pair matches the exclusion filter or matches no filters, true - if pair matches in the inclusion filter public bool InstrumentClass(string processName, string assemblyName, string className) { if (string.IsNullOrEmpty(processName) || string.IsNullOrEmpty(assemblyName) || string.IsNullOrEmpty(className)) @@ -96,11 +113,23 @@ public bool InstrumentClass(string processName, string assemblyName, string clas } + /// + /// Determine if an [assemblyname]classname pair matches the current Exclusion or Inclusion filters + /// + /// the name of the assembly under profile + /// the name of the class under profile + /// false - if pair matches the exclusion filter or matches no filters, true - if pair matches in the inclusion filter public bool InstrumentClass(string assemblyName, string className) { return InstrumentClass(Guid.NewGuid().ToString(), assemblyName, className); } + /// + /// Add a filter + /// + /// A filter is of the format (+ or -)[assemblyName]className, wildcards are allowed.
+ /// i.e. -[mscorlib], -[System.*]*, +[App.*]*, +[*]* + /// public void AddFilter(string assemblyClassName) { string assemblyName; @@ -154,11 +183,20 @@ private static InvalidOperationException InvalidFilterFormatException(string ass return new InvalidOperationException(string.Format("The supplied filter '{0}' does not meet the required format for a filter +-[assemblyname]classname", assemblyClassName)); } + /// + /// Add attribute exclusion filters + /// + /// An array of filters that are used to wildcard match an attribute public void AddAttributeExclusionFilters(string[] exclusionFilters) { ExcludedAttributes.AddFilters(exclusionFilters, RegExFilters); } + /// + /// Is this entity (method/type) excluded due to an attributeFilter + /// + /// The entity to test + /// public bool ExcludeByAttribute(IMemberDefinition entity) { if (ExcludedAttributes.Count == 0) @@ -207,6 +245,11 @@ where excludeAttribute.IsMatchingExpression(customAttribute.AttributeType.FullNa select excludeAttribute).Any(); } + /// + /// Is this entity excluded due to an attributeFilter + /// + /// The entity to test + /// public bool ExcludeByAttribute(AssemblyDefinition entity) { if (ExcludedAttributes.Count == 0) @@ -215,6 +258,11 @@ public bool ExcludeByAttribute(AssemblyDefinition entity) return ExcludeByAttribute((ICustomAttributeProvider)entity); } + /// + /// Is this file excluded + /// + /// The name of the file to test + /// public bool ExcludeByFile(string fileName) { if (ExcludedFiles.Count == 0 || string.IsNullOrWhiteSpace(fileName)) @@ -223,11 +271,20 @@ public bool ExcludeByFile(string fileName) return ExcludedFiles.Any(excludeFile => excludeFile.IsMatchingExpression(fileName)); } + /// + /// Add file exclusion filters + /// + /// public void AddFileExclusionFilters(string[] exclusionFilters) { ExcludedFiles.AddFilters(exclusionFilters, RegExFilters); } + /// + /// Decides whether an assembly should be analysed for test methods + /// + /// the name of the assembly under profile + /// true - if the assembly matches the test assembly filter public bool UseTestAssembly(string assemblyName) { if (TestFiles.Count == 0 || string.IsNullOrWhiteSpace(assemblyName)) @@ -236,11 +293,20 @@ public bool UseTestAssembly(string assemblyName) return TestFiles.Any(file => file.IsMatchingExpression(assemblyName)); } + /// + /// Add test file filters + /// + /// public void AddTestFileFilters(string[] testFilters) { TestFiles.AddFilters(testFilters, RegExFilters); } + /// + /// Is the method an auto-implemented property get/set + /// + /// + /// public bool IsAutoImplementedProperty(MethodDefinition method) { if ((method.IsSetter || method.IsGetter) && method.HasCustomAttributes) @@ -250,6 +316,11 @@ public bool IsAutoImplementedProperty(MethodDefinition method) return false; } + /// + /// Should we instrument this asssembly + /// + /// + /// public bool InstrumentProcess(string processName) { if (string.IsNullOrEmpty(processName)) diff --git a/main/OpenCover.Framework/Manager/MemoryManager.cs b/main/OpenCover.Framework/Manager/MemoryManager.cs index c97604ef1..b034b849c 100644 --- a/main/OpenCover.Framework/Manager/MemoryManager.cs +++ b/main/OpenCover.Framework/Manager/MemoryManager.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.IO; using System.IO.MemoryMappedFiles; using System.Linq; @@ -28,7 +27,12 @@ public class MemoryManager : IMemoryManager /// public class ManagedBlock { + /// + /// protected string Namespace; + + /// + /// protected string Key; /// @@ -343,6 +347,10 @@ public void RemoveDeactivatedBlocks() } } + /// + /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources. + /// + /// 2 public void Dispose() { lock (_lockObject) diff --git a/main/OpenCover.Framework/Manager/ProfilerManager.cs b/main/OpenCover.Framework/Manager/ProfilerManager.cs index e8a8e5d72..106ea3113 100644 --- a/main/OpenCover.Framework/Manager/ProfilerManager.cs +++ b/main/OpenCover.Framework/Manager/ProfilerManager.cs @@ -228,15 +228,22 @@ private void ProcessMessages(WaitHandle[] handles) .Select(g => g.Select(a => a.Pair).ToList()) .Select(g => Task.Factory.StartNew(() => { - g.Select(h => h.Item1).ToList().ForEach(h => h.Set()); - WaitHandle.WaitAll(g.Select(h => h.Item2).ToArray(), new TimeSpan(0, 0, 20)); + ConsumeException(() => + { + g.Select(h => h.Item1).ToList().ForEach(h => h.Set()); + WaitHandle.WaitAll(g.Select(h => h.Item2).ToArray(), new TimeSpan(0, 0, 20)); + }); })).ToArray(); Task.WaitAll(tasks); foreach (var threadHandle in threadHandles) { - threadHandle.Item1.Dispose(); - threadHandle.Item2.Dispose(); + var handle = threadHandle; + ConsumeException(() => + { + handle.Item1.Dispose(); + handle.Item2.Dispose(); + }); } threadHandles.Clear(); } @@ -245,6 +252,18 @@ private void ProcessMessages(WaitHandle[] handles) _messageQueue.Enqueue(new byte[0]); } + // wrap exceptions when closing down + private static void ConsumeException(Action doSomething) + { + try + { + doSomething(); + } + catch (Exception ex) + { + DebugLogger.Error("An unexpected exception was encountered but consumed.", ex); + } + } private Tuple StartProcessingThread(ManagedBufferBlock block) { DebugLogger.InfoFormat("Starting Process Block => {0}", block.BufferId); diff --git a/main/OpenCover.Framework/Model/Class.cs b/main/OpenCover.Framework/Model/Class.cs index c7df3e252..9e4167973 100644 --- a/main/OpenCover.Framework/Model/Class.cs +++ b/main/OpenCover.Framework/Model/Class.cs @@ -37,6 +37,10 @@ public Class() /// public Method[] Methods { get; set; } + /// + /// If a class was skipped by instrumentation, supply the reason why + /// + /// public override void MarkAsSkipped(SkippedMethod reason) { SkippedDueTo = reason; diff --git a/main/OpenCover.Framework/Model/File.cs b/main/OpenCover.Framework/Model/File.cs index b21492d84..002d94377 100644 --- a/main/OpenCover.Framework/Model/File.cs +++ b/main/OpenCover.Framework/Model/File.cs @@ -6,7 +6,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Text; using System.Threading; using System.Xml.Serialization; @@ -17,6 +16,9 @@ namespace OpenCover.Framework.Model /// public class FileRef { + /// + /// The uniqueid of a file in a coverage session + /// [XmlAttribute("uid")] public UInt32 UniqueId { get; set; } } diff --git a/main/OpenCover.Framework/Model/InstrumentationModelBuilderFactory.cs b/main/OpenCover.Framework/Model/InstrumentationModelBuilderFactory.cs index 160356ea9..470ab72e4 100644 --- a/main/OpenCover.Framework/Model/InstrumentationModelBuilderFactory.cs +++ b/main/OpenCover.Framework/Model/InstrumentationModelBuilderFactory.cs @@ -35,6 +35,12 @@ public InstrumentationModelBuilderFactory(ICommandLine commandLine, IFilter filt _trackedMethodStrategyManager = trackedMethodStrategyManager; } + /// + /// Create a Model Builder for a module + /// + /// + /// + /// public IInstrumentationModelBuilder CreateModelBuilder(string modulePath, string moduleName) { var manager = new CecilSymbolManager(_commandLine, _filter, _logger, _trackedMethodStrategyManager); diff --git a/main/OpenCover.Framework/Model/Method.cs b/main/OpenCover.Framework/Model/Method.cs index afb790437..5760981a7 100644 --- a/main/OpenCover.Framework/Model/Method.cs +++ b/main/OpenCover.Framework/Model/Method.cs @@ -3,7 +3,7 @@ // // This source code is released under the MIT License; see the accompanying license file. // -using System.Collections.Generic; + using System.Xml.Serialization; namespace OpenCover.Framework.Model @@ -95,6 +95,10 @@ public class Method : SummarySkippedEntity [XmlAttribute("isSetter")] public bool IsSetter { get; set; } + /// + /// Mark an entity as skipped + /// + /// Provide a reason public override void MarkAsSkipped(SkippedMethod reason) { SkippedDueTo = reason; diff --git a/main/OpenCover.Framework/Model/Module.cs b/main/OpenCover.Framework/Model/Module.cs index c6817e602..0a3b2bd6e 100644 --- a/main/OpenCover.Framework/Model/Module.cs +++ b/main/OpenCover.Framework/Model/Module.cs @@ -3,10 +3,8 @@ // // This source code is released under the MIT License; see the accompanying license file. // -using System; + using System.Collections.Generic; -using System.Linq; -using System.Text; using System.Xml.Serialization; namespace OpenCover.Framework.Model @@ -61,6 +59,10 @@ public Module() [XmlAttribute("hash")] public string ModuleHash { get; set; } + /// + /// Mark an entity as skipped + /// + /// Provide a reason public override void MarkAsSkipped(SkippedMethod reason) { SkippedDueTo = reason; diff --git a/main/OpenCover.Framework/Model/SequencePoint.cs b/main/OpenCover.Framework/Model/SequencePoint.cs index db19eb4a0..8a6d7e32f 100644 --- a/main/OpenCover.Framework/Model/SequencePoint.cs +++ b/main/OpenCover.Framework/Model/SequencePoint.cs @@ -38,9 +38,15 @@ public class SequencePoint : InstrumentationPoint, IDocumentReference [XmlAttribute("ec")] public int EndColumn { get; set; } + /// + /// The number of branch exits + /// [XmlAttribute("bec")] public int BranchExitsCount { get; set; } + /// + /// The number of times the branch exists were visited + /// [XmlAttribute("bev")] public int BranchExitsVisit { get; set; } diff --git a/main/OpenCover.Framework/Model/SkippedEntity.cs b/main/OpenCover.Framework/Model/SkippedEntity.cs index ed771710c..c447ea754 100644 --- a/main/OpenCover.Framework/Model/SkippedEntity.cs +++ b/main/OpenCover.Framework/Model/SkippedEntity.cs @@ -7,7 +7,7 @@ namespace OpenCover.Framework.Model /// public abstract class SkippedEntity { - private SkippedMethod? skippedDueTo; + private SkippedMethod? _skippedDueTo; /// /// If this class has been skipped then this value will describe why @@ -15,14 +15,14 @@ public abstract class SkippedEntity [XmlAttribute("skippedDueTo")] public SkippedMethod SkippedDueTo { - get { return skippedDueTo.GetValueOrDefault(); } - set { skippedDueTo = value; } + get { return _skippedDueTo.GetValueOrDefault(); } + set { _skippedDueTo = value; } } /// /// If this class has been skipped then this value will allow the data to be serialized /// - public bool ShouldSerializeSkippedDueTo() { return skippedDueTo.HasValue; } + public bool ShouldSerializeSkippedDueTo() { return _skippedDueTo.HasValue; } /// /// Mark an entity as skipped diff --git a/main/OpenCover.Framework/Model/SummarySkippedEntity.cs b/main/OpenCover.Framework/Model/SummarySkippedEntity.cs index 1c305c7f7..42f2bbccd 100644 --- a/main/OpenCover.Framework/Model/SummarySkippedEntity.cs +++ b/main/OpenCover.Framework/Model/SummarySkippedEntity.cs @@ -6,6 +6,9 @@ namespace OpenCover.Framework.Model /// public abstract class SummarySkippedEntity : SkippedEntity { + /// + /// Initialise + /// protected SummarySkippedEntity() { Summary = new Summary(); diff --git a/main/OpenCover.Framework/Model/TrackedMethod.cs b/main/OpenCover.Framework/Model/TrackedMethod.cs index f73b3f99c..3a742b84f 100644 --- a/main/OpenCover.Framework/Model/TrackedMethod.cs +++ b/main/OpenCover.Framework/Model/TrackedMethod.cs @@ -18,7 +18,9 @@ public class TrackedMethodRef [XmlAttribute("uid")] public UInt32 UniqueId { get; set; } - // visit count + /// + /// The visit count + /// [XmlAttribute("vc")] public int VisitCount { get; set; } diff --git a/main/OpenCover.Framework/Persistance/BasePersistance.cs b/main/OpenCover.Framework/Persistance/BasePersistance.cs index d09e25b1b..ad3e0f892 100644 --- a/main/OpenCover.Framework/Persistance/BasePersistance.cs +++ b/main/OpenCover.Framework/Persistance/BasePersistance.cs @@ -12,6 +12,9 @@ namespace OpenCover.Framework.Persistance /// public abstract class BasePersistance : IPersistance { + /// + /// Provides subclasses access to the command line object + /// protected readonly ICommandLine CommandLine; private readonly ILog _logger; private uint _trackedMethodId; diff --git a/main/OpenCover.Framework/Persistance/FilePersistance.cs b/main/OpenCover.Framework/Persistance/FilePersistance.cs index ffc063810..f1c3cb2c8 100644 --- a/main/OpenCover.Framework/Persistance/FilePersistance.cs +++ b/main/OpenCover.Framework/Persistance/FilePersistance.cs @@ -68,6 +68,9 @@ private void LoadCoverageFile() } } + /// + /// we are done and the data needs one last clean up + /// public override void Commit() { _logger.Info("Committing..."); diff --git a/main/OpenCover.Framework/Strategy/TrackedMethodStrategyManager.cs b/main/OpenCover.Framework/Strategy/TrackedMethodStrategyManager.cs index a5677b87e..a50678ab8 100644 --- a/main/OpenCover.Framework/Strategy/TrackedMethodStrategyManager.cs +++ b/main/OpenCover.Framework/Strategy/TrackedMethodStrategyManager.cs @@ -66,6 +66,12 @@ public TrackedMethodStrategyManager() } private int _methodId; + + /// + /// Get the tracked methods for the target assembly + /// + /// + /// public TrackedMethod[] GetTrackedMethods(string assembly) { var methods = _proxy.GetTrackedMethods(assembly); @@ -76,6 +82,10 @@ public TrackedMethod[] GetTrackedMethods(string assembly) return methods; } + /// + /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources. + /// + /// 2 public void Dispose() { _proxy = null; diff --git a/main/OpenCover.Framework/Utility/PerfCounters.cs b/main/OpenCover.Framework/Utility/PerfCounters.cs index e1774b247..a8feb4832 100644 --- a/main/OpenCover.Framework/Utility/PerfCounters.cs +++ b/main/OpenCover.Framework/Utility/PerfCounters.cs @@ -9,15 +9,19 @@ namespace OpenCover.Framework.Utility public class PerfCounters : IPerfCounters { private PerformanceCounter _memoryQueue; - private PerformanceCounter _queueThrougput; + private PerformanceCounter _queueThroughput; /// /// get the current queue size /// public int CurrentMemoryQueueSize { set { _memoryQueue.RawValue = value; } } + + /// + /// Increment the block size + /// public void IncrementBlocksReceived() { - _queueThrougput.RawValue += 1; + _queueThroughput.RawValue += 1; } /// @@ -50,13 +54,16 @@ private void CreateCounters() PerformanceCounterCategoryType.SingleInstance, counters); _memoryQueue = new PerformanceCounter(CategoryName, MemoryQueue, false) { RawValue = 0 }; - _queueThrougput = new PerformanceCounter(CategoryName, QueueThroughput, false) { RawValue = 0 }; + _queueThroughput = new PerformanceCounter(CategoryName, QueueThroughput, false) { RawValue = 0 }; } + /// + /// Reset all counters + /// public void ResetCounters() { _memoryQueue.RawValue = 0; - _queueThrougput.RawValue = 0; + _queueThroughput.RawValue = 0; } } @@ -69,11 +76,19 @@ public class NullPerfCounter : IPerfCounters /// /// A null performance counters implementation /// + // ReSharper disable once UnusedAutoPropertyAccessor.Local public int CurrentMemoryQueueSize { set; private get; } + + /// + /// Increment the number of blocks received + /// public void IncrementBlocksReceived() { } + /// + /// Reset all counters + /// public void ResetCounters() { } From f463cc216e506c05aed8a76a85dc8eb06778a42c Mon Sep 17 00:00:00 2001 From: sawilde Date: Tue, 1 Dec 2015 20:58:15 +1100 Subject: [PATCH 03/10] #335 allow short wait time to be configured more housekeeping --- main/OpenCover.Profiler/CodeCoverage.cpp | 90 ++--- main/OpenCover.Profiler/CodeCoverage.h | 5 +- .../CodeCoverage_Callback.cpp | 8 +- .../CodeCoverage_Thread.cpp | 4 +- .../ProfilerCommunication.cpp | 344 +++++++++--------- .../ProfilerCommunication.h | 49 +-- 6 files changed, 256 insertions(+), 244 deletions(-) diff --git a/main/OpenCover.Profiler/CodeCoverage.cpp b/main/OpenCover.Profiler/CodeCoverage.cpp index 79718f342..d16f9b25b 100644 --- a/main/OpenCover.Profiler/CodeCoverage.cpp +++ b/main/OpenCover.Profiler/CodeCoverage.cpp @@ -10,7 +10,7 @@ #include "NativeCallback.h" #include "dllmain.h" -CCodeCoverage* CCodeCoverage::g_pProfiler = NULL; +CCodeCoverage* CCodeCoverage::g_pProfiler = nullptr; // CCodeCoverage /// Handle ICorProfilerCallback::Initialize @@ -30,7 +30,7 @@ HRESULT CCodeCoverage::OpenCoverInitialise(IUnknown *pICorProfilerInfoUnk){ //::OutputDebugStringW(szGuid); WCHAR szExeName[MAX_PATH]; - GetModuleFileNameW(NULL, szExeName, MAX_PATH); + GetModuleFileNameW(nullptr, szExeName, MAX_PATH); RELTRACE(L" ::Initialize(...) => EXE = %s", szExeName); WCHAR szModuleName[MAX_PATH]; @@ -38,31 +38,31 @@ HRESULT CCodeCoverage::OpenCoverInitialise(IUnknown *pICorProfilerInfoUnk){ RELTRACE(L" ::Initialize(...) => PROFILER = %s", szModuleName); //::OutputDebugStringW(szModuleName); - if (g_pProfiler!=NULL) + if (g_pProfiler!=nullptr) RELTRACE(_T("Another instance of the profiler is running under this process...")); m_profilerInfo = pICorProfilerInfoUnk; - if (m_profilerInfo != NULL) ATLTRACE(_T(" ::Initialize (m_profilerInfo OK)")); - if (m_profilerInfo == NULL) return E_FAIL; + if (m_profilerInfo != nullptr) ATLTRACE(_T(" ::Initialize (m_profilerInfo OK)")); + if (m_profilerInfo == nullptr) return E_FAIL; m_profilerInfo2 = pICorProfilerInfoUnk; - if (m_profilerInfo2 != NULL) ATLTRACE(_T(" ::Initialize (m_profilerInfo2 OK)")); - if (m_profilerInfo2 == NULL) return E_FAIL; + if (m_profilerInfo2 != nullptr) ATLTRACE(_T(" ::Initialize (m_profilerInfo2 OK)")); + if (m_profilerInfo2 == nullptr) return E_FAIL; m_profilerInfo3 = pICorProfilerInfoUnk; #ifndef _TOOLSETV71 m_profilerInfo4 = pICorProfilerInfoUnk; #endif ZeroMemory(&m_runtimeVersion, sizeof(m_runtimeVersion)); - if (m_profilerInfo3 != NULL) + if (m_profilerInfo3 != nullptr) { ATLTRACE(_T(" ::Initialize (m_profilerInfo3 OK)")); ZeroMemory(&m_runtimeVersion, sizeof(m_runtimeVersion)); - m_profilerInfo3->GetRuntimeInformation(NULL, &m_runtimeType, + m_profilerInfo3->GetRuntimeInformation(nullptr, &m_runtimeType, &m_runtimeVersion.usMajorVersion, &m_runtimeVersion.usMinorVersion, &m_runtimeVersion.usBuildNumber, - &m_runtimeVersion.usRevisionNumber, 0, NULL, NULL); + &m_runtimeVersion.usRevisionNumber, 0, nullptr, nullptr); ATLTRACE(_T(" ::Initialize (Runtime %d)"), m_runtimeType); } @@ -81,7 +81,7 @@ HRESULT CCodeCoverage::OpenCoverInitialise(IUnknown *pICorProfilerInfoUnk){ TCHAR threshold[1024] = {0}; ::GetEnvironmentVariable(_T("OpenCover_Profiler_Threshold"), threshold, 1024); - m_threshold = _tcstoul(threshold, NULL, 10); + m_threshold = _tcstoul(threshold, nullptr, 10); ATLTRACE(_T(" ::Initialize(...) => threshold = %ul"), m_threshold); TCHAR tracebyTest[1024] = {0}; @@ -89,10 +89,21 @@ HRESULT CCodeCoverage::OpenCoverInitialise(IUnknown *pICorProfilerInfoUnk){ m_tracingEnabled = _tcslen(tracebyTest) != 0; ATLTRACE(_T(" ::Initialize(...) => tracingEnabled = %s (%s)"), m_tracingEnabled ? _T("true") : _T("false"), tracebyTest); + TCHAR shortwait[1024] = { 0 }; + if (::GetEnvironmentVariable(_T("OpenCover_Profiler_ShortWait"), shortwait, 1024) > 0) { + _shortwait = _tcstoul(shortwait, nullptr, 10); + if (_shortwait < 10000) + _shortwait = 10000; + if (_shortwait > 60000) + _shortwait = 60000; + ATLTRACE(_T(" ::Initialize(...) => shortwait = %ul"), _shortwait); + } m_useOldStyle = (tstring(instrumentation) == _T("oldSchool")); - if (!m_host.Initialise(key, ns, szExeName)) + _host = std::make_shared(ProfilerCommunication(_shortwait)); + + if (!_host->Initialise(key, ns, szExeName)) { RELTRACE(_T(" ::Initialize => Profiler will not run for this process.")); return E_FAIL; @@ -100,14 +111,14 @@ HRESULT CCodeCoverage::OpenCoverInitialise(IUnknown *pICorProfilerInfoUnk){ OpenCoverSupportInitialize(pICorProfilerInfoUnk); - if (m_chainedProfiler == NULL){ + if (m_chainedProfiler == nullptr){ DWORD dwMask = AppendProfilerEventMask(0); COM_FAIL_MSG_RETURN_ERROR(m_profilerInfo2->SetEventMask(dwMask), _T(" ::Initialize(...) => SetEventMask => 0x%X")); } - if(m_profilerInfo3 != NULL) + if(m_profilerInfo3 != nullptr) { COM_FAIL_MSG_RETURN_ERROR(m_profilerInfo3->SetFunctionIDMapper2(FunctionMapper2, this), _T(" ::Initialize(...) => SetFunctionIDMapper2 => 0x%X")); @@ -146,7 +157,7 @@ DWORD CCodeCoverage::AppendProfilerEventMask(DWORD currentEventMask) dwMask |= COR_PRF_DISABLE_TRANSPARENCY_CHECKS_UNDER_FULL_TRUST; // Disables security transparency checks that are normally done during just-in-time (JIT) compilation and class loading for full-trust assemblies. This can make some instrumentation easier to perform. #ifndef _TOOLSETV71 - if (m_profilerInfo4 != NULL) + if (m_profilerInfo4 != nullptr) { ATLTRACE(_T(" ::Initialize (m_profilerInfo4 OK)")); dwMask |= COR_PRF_DISABLE_ALL_NGEN_IMAGES; @@ -163,15 +174,15 @@ HRESULT STDMETHODCALLTYPE CCodeCoverage::Shutdown( void) { RELTRACE(_T("::Shutdown - Starting")); - if (m_chainedProfiler != NULL) + if (m_chainedProfiler != nullptr) m_chainedProfiler->Shutdown(); - m_host.CloseChannel(m_tracingEnabled); + _host->CloseChannel(m_tracingEnabled); WCHAR szExeName[MAX_PATH]; - GetModuleFileNameW(NULL, szExeName, MAX_PATH); + GetModuleFileNameW(nullptr, szExeName, MAX_PATH); RELTRACE(_T("::Shutdown - Nothing left to do but return S_OK(%s)"), szExeName); - g_pProfiler = NULL; + g_pProfiler = nullptr; return S_OK; } @@ -197,10 +208,10 @@ void __fastcall CCodeCoverage::AddVisitPoint(ULONG uniqueId) } if (m_tracingEnabled){ - m_host.AddVisitPoint(uniqueId); + _host->AddVisitPoint(uniqueId); } else { - m_host.AddVisitPointToThreadBuffer(uniqueId, IT_VisitPoint); + _host->AddVisitPointToThreadBuffer(uniqueId, IT_VisitPoint); } } @@ -215,7 +226,7 @@ HRESULT STDMETHODCALLTYPE CCodeCoverage::ModuleLoadFinished( /* [in] */ ModuleID moduleId, /* [in] */ HRESULT hrStatus) { - if (m_chainedProfiler != NULL) + if (m_chainedProfiler != nullptr) m_chainedProfiler->ModuleLoadFinished(moduleId, hrStatus); return RegisterCuckoos(moduleId); @@ -228,7 +239,7 @@ HRESULT STDMETHODCALLTYPE CCodeCoverage::ModuleAttachedToAssembly( /* [in] */ ModuleID moduleId, /* [in] */ AssemblyID assemblyId) { - if (m_chainedProfiler != NULL) + if (m_chainedProfiler != nullptr) m_chainedProfiler->ModuleAttachedToAssembly(moduleId, assemblyId); std::wstring modulePath = GetModulePath(moduleId); @@ -236,7 +247,7 @@ HRESULT STDMETHODCALLTYPE CCodeCoverage::ModuleAttachedToAssembly( /*ATLTRACE(_T("::ModuleAttachedToAssembly(...) => (%X => %s, %X => %s)"), moduleId, W2CT(modulePath.c_str()), assemblyId, W2CT(assemblyName.c_str()));*/ - m_allowModules[modulePath] = m_host.TrackAssembly((LPWSTR)modulePath.c_str(), (LPWSTR)assemblyName.c_str()); + m_allowModules[modulePath] = _host->TrackAssembly(const_cast(modulePath.c_str()), const_cast(assemblyName.c_str())); m_allowModulesAssemblyMap[modulePath] = assemblyName; if (m_allowModules[modulePath]){ @@ -273,19 +284,17 @@ HRESULT STDMETHODCALLTYPE CCodeCoverage::JITCompilationStarted( std::vector seqPoints; std::vector brPoints; - if (m_host.GetPoints(functionToken, (LPWSTR)modulePath.c_str(), - (LPWSTR)m_allowModulesAssemblyMap[modulePath].c_str(), seqPoints, brPoints)) + if (_host->GetPoints(functionToken, const_cast(modulePath.c_str()), + const_cast(m_allowModulesAssemblyMap[modulePath].c_str()), seqPoints, brPoints)) { if (seqPoints.size() != 0) { - LPCBYTE pMethodHeader = NULL; + IMAGE_COR_ILMETHOD* pMethodHeader = nullptr; ULONG iMethodSize = 0; - COM_FAIL_MSG_RETURN_ERROR(m_profilerInfo2->GetILFunctionBody(moduleId, functionToken, &pMethodHeader, &iMethodSize), + COM_FAIL_MSG_RETURN_ERROR(m_profilerInfo2->GetILFunctionBody(moduleId, functionToken, (LPCBYTE*)&pMethodHeader, &iMethodSize), _T(" ::JITCompilationStarted(...) => GetILFunctionBody => 0x%X")); - IMAGE_COR_ILMETHOD* pMethod = (IMAGE_COR_ILMETHOD*)pMethodHeader; - - Method instumentedMethod(pMethod); + Method instumentedMethod(pMethodHeader); instumentedMethod.IncrementStackSize(2); ATLTRACE(_T("::JITCompilationStarted(...) => Instrumenting...")); @@ -301,13 +310,13 @@ HRESULT STDMETHODCALLTYPE CCodeCoverage::JITCompilationStarted( COM_FAIL_MSG_RETURN_ERROR(m_profilerInfo2->GetILFunctionBodyAllocator(moduleId, &methodMalloc), _T(" ::JITCompilationStarted(...) => GetILFunctionBodyAllocator=> 0x%X")); - IMAGE_COR_ILMETHOD* pNewMethod = (IMAGE_COR_ILMETHOD*)methodMalloc->Alloc(instumentedMethod.GetMethodSize()); + auto pNewMethod = static_cast(methodMalloc->Alloc(instumentedMethod.GetMethodSize())); instumentedMethod.WriteMethod(pNewMethod); COM_FAIL_MSG_RETURN_ERROR(m_profilerInfo2->SetILFunctionBody(moduleId, functionToken, (LPCBYTE)pNewMethod), _T(" ::JITCompilationStarted(...) => SetILFunctionBody => 0x%X")); ULONG mapSize = instumentedMethod.GetILMapSize(); - COR_IL_MAP * pMap = (COR_IL_MAP *)CoTaskMemAlloc(mapSize * sizeof(COR_IL_MAP)); + COR_IL_MAP * pMap = static_cast(CoTaskMemAlloc(mapSize * sizeof(COR_IL_MAP))); instumentedMethod.PopulateILMap(mapSize, pMap); COM_FAIL_MSG_RETURN_ERROR(m_profilerInfo2->SetILInstrumentedCodeMap(functionId, TRUE, mapSize, pMap), _T(" ::JITCompilationStarted(...) => SetILInstrumentedCodeMap => 0x%X")); @@ -329,7 +338,7 @@ HRESULT STDMETHODCALLTYPE CCodeCoverage::JITCompilationStarted( } } - if (m_chainedProfiler != NULL) + if (m_chainedProfiler != nullptr) return m_chainedProfiler->JITCompilationStarted(functionId, fIsSafeToBlock); return S_OK; @@ -343,8 +352,8 @@ void CCodeCoverage::InstrumentMethod(ModuleID moduleId, Method& method, std::ve { if (m_useOldStyle) { - mdSignature pvsig = GetMethodSignatureToken_I4(moduleId); - void(__fastcall *pt)(ULONG) = GetInstrumentPointVisit(); + auto pvsig = GetMethodSignatureToken_I4(moduleId); + auto pt = GetInstrumentPointVisit(); InstructionList instructions; if (seqPoints.size() > 0) @@ -384,13 +393,12 @@ void CCodeCoverage::InstrumentMethod(ModuleID moduleId, Method& method, std::ve HRESULT CCodeCoverage::InstrumentMethodWith(ModuleID moduleId, mdToken functionToken, InstructionList &instructions){ - LPCBYTE pMethodHeader = NULL; + IMAGE_COR_ILMETHOD* pMethodHeader = nullptr; ULONG iMethodSize = 0; - COM_FAIL_MSG_RETURN_ERROR(m_profilerInfo->GetILFunctionBody(moduleId, functionToken, &pMethodHeader, &iMethodSize), + COM_FAIL_MSG_RETURN_ERROR(m_profilerInfo->GetILFunctionBody(moduleId, functionToken, (LPCBYTE*)&pMethodHeader, &iMethodSize), _T(" ::InstrumentMethodWith(...) => GetILFunctionBody => 0x%X")); - IMAGE_COR_ILMETHOD* pMethod = (IMAGE_COR_ILMETHOD*)pMethodHeader; - Method instumentedMethod(pMethod); + Method instumentedMethod(pMethodHeader); instumentedMethod.InsertInstructionsAtOriginalOffset(0, instructions); @@ -401,7 +409,7 @@ HRESULT CCodeCoverage::InstrumentMethodWith(ModuleID moduleId, mdToken functionT COM_FAIL_MSG_RETURN_ERROR(m_profilerInfo->GetILFunctionBodyAllocator(moduleId, &methodMalloc), _T(" ::InstrumentMethodWith(...) => GetILFunctionBodyAllocator=> 0x%X")); - IMAGE_COR_ILMETHOD* pNewMethod = (IMAGE_COR_ILMETHOD*)methodMalloc->Alloc(instumentedMethod.GetMethodSize()); + IMAGE_COR_ILMETHOD* pNewMethod = static_cast(methodMalloc->Alloc(instumentedMethod.GetMethodSize())); instumentedMethod.WriteMethod(pNewMethod); COM_FAIL_MSG_RETURN_ERROR(m_profilerInfo->SetILFunctionBody(moduleId, functionToken, (LPCBYTE)pNewMethod), _T(" ::InstrumentMethodWith(...) => SetILFunctionBody => 0x%X")); diff --git a/main/OpenCover.Profiler/CodeCoverage.h b/main/OpenCover.Profiler/CodeCoverage.h index a5241f3ab..18dd148ee 100644 --- a/main/OpenCover.Profiler/CodeCoverage.h +++ b/main/OpenCover.Profiler/CodeCoverage.h @@ -17,6 +17,7 @@ #include #include "ReleaseTrace.h" +#include using namespace ATL; @@ -52,6 +53,7 @@ class ATL_NO_VTABLE CCodeCoverage : m_cuckooCriticalToken = 0; m_cuckooSafeToken = 0; m_infoHook = nullptr; + _shortwait = 10000; } DECLARE_REGISTRY_RESOURCEID(IDR_CODECOVERAGE) @@ -99,7 +101,8 @@ END_COM_MAP() void __fastcall AddVisitPoint(ULONG uniqueId); private: - ProfilerCommunication m_host; + std::shared_ptr _host; + ULONG _shortwait; HRESULT OpenCoverInitialise(IUnknown *pICorProfilerInfoUnk); DWORD AppendProfilerEventMask(DWORD currentEventMask); diff --git a/main/OpenCover.Profiler/CodeCoverage_Callback.cpp b/main/OpenCover.Profiler/CodeCoverage_Callback.cpp index bfbdd4be5..d92b0b59f 100644 --- a/main/OpenCover.Profiler/CodeCoverage_Callback.cpp +++ b/main/OpenCover.Profiler/CodeCoverage_Callback.cpp @@ -24,7 +24,7 @@ UINT_PTR CCodeCoverage::FunctionMapper2(FunctionID functionId, void* clientData, if (profiler->GetTokenAndModule(functionId, functionToken, moduleId, modulePath, &assemblyId)) { ULONG uniqueId; - if (profiler->m_host.TrackMethod(functionToken, (LPWSTR)modulePath.c_str(), + if (profiler->_host->TrackMethod(functionToken, (LPWSTR)modulePath.c_str(), (LPWSTR)profiler->m_allowModulesAssemblyMap[modulePath].c_str(), uniqueId)) { *pbHookFunction = TRUE; @@ -46,7 +46,7 @@ void CCodeCoverage::FunctionEnter2( /*[in]*/COR_PRF_FRAME_INFO func, /*[in]*/COR_PRF_FUNCTION_ARGUMENT_INFO *argumentInfo) { - m_host.AddTestEnterPoint((ULONG)clientData); + _host->AddTestEnterPoint((ULONG)clientData); } void CCodeCoverage::FunctionLeave2( @@ -55,7 +55,7 @@ void CCodeCoverage::FunctionLeave2( /*[in]*/COR_PRF_FRAME_INFO func, /*[in]*/COR_PRF_FUNCTION_ARGUMENT_RANGE *retvalRange) { - m_host.AddTestLeavePoint((ULONG)clientData); + _host->AddTestLeavePoint((ULONG)clientData); } void CCodeCoverage::FunctionTailcall2( @@ -63,5 +63,5 @@ void CCodeCoverage::FunctionTailcall2( /*[in]*/UINT_PTR clientData, /*[in]*/COR_PRF_FRAME_INFO func) { - m_host.AddTestTailcallPoint((ULONG)clientData); + _host->AddTestTailcallPoint((ULONG)clientData); } diff --git a/main/OpenCover.Profiler/CodeCoverage_Thread.cpp b/main/OpenCover.Profiler/CodeCoverage_Thread.cpp index 62b37ec1e..18b9c7192 100644 --- a/main/OpenCover.Profiler/CodeCoverage_Thread.cpp +++ b/main/OpenCover.Profiler/CodeCoverage_Thread.cpp @@ -21,7 +21,7 @@ HRESULT STDMETHODCALLTYPE CCodeCoverage::ThreadDestroyed( m_chainedProfiler->ThreadDestroyed(threadId); if (!m_tracingEnabled){ - m_host.ThreadDestroyed(threadId); + _host->ThreadDestroyed(threadId); } return S_OK; @@ -36,7 +36,7 @@ HRESULT STDMETHODCALLTYPE CCodeCoverage::ThreadAssignedToOSThread( m_chainedProfiler->ThreadAssignedToOSThread(managedThreadId, osThreadId); if (!m_tracingEnabled){ - m_host.ThreadCreated(managedThreadId, osThreadId); + _host->ThreadCreated(managedThreadId, osThreadId); } return S_OK; diff --git a/main/OpenCover.Profiler/ProfilerCommunication.cpp b/main/OpenCover.Profiler/ProfilerCommunication.cpp index 620ba9583..e240b520e 100644 --- a/main/OpenCover.Profiler/ProfilerCommunication.cpp +++ b/main/OpenCover.Profiler/ProfilerCommunication.cpp @@ -13,16 +13,16 @@ #include #define ONERROR_GOEXIT(hr) if (FAILED(hr)) goto Exit -#define COMM_WAIT_SHORT 10000 -#define COMM_WAIT_LONG 60000 -#define COMM_WAIT_VSHORT 3000 +#define COM_WAIT_LONG 60000 +#define COM_WAIT_VSHORT 3000 -ProfilerCommunication::ProfilerCommunication() +ProfilerCommunication::ProfilerCommunication(DWORD short_wait) { - m_bufferId = 0; - m_pMSG = nullptr; - m_pVisitPoints = nullptr; - hostCommunicationActive = false; + _bufferId = 0; + _pMSG = nullptr; + _pVisitPoints = nullptr; + _hostCommunicationActive = false; + _short_wait = short_wait; } ProfilerCommunication::~ProfilerCommunication() @@ -31,57 +31,57 @@ ProfilerCommunication::~ProfilerCommunication() bool ProfilerCommunication::Initialise(TCHAR *key, TCHAR *ns, TCHAR *processName) { - m_key = key; - m_processName = processName; + _key = key; + _processName = processName; std::wstring sharedKey = key; sharedKey.append(_T("-1")); - m_namespace = ns; + _namespace = ns; - m_mutexCommunication.Initialise((m_namespace + _T("\\OpenCover_Profiler_Communication_Mutex_") + m_key).c_str()); - if (!m_mutexCommunication.IsValid()) return false; + _mutexCommunication.Initialise((_namespace + _T("\\OpenCover_Profiler_Communication_Mutex_") + _key).c_str()); + if (!_mutexCommunication.IsValid()) return false; USES_CONVERSION; ATLTRACE(_T("ProfilerCommunication::Initialise(...) => Initialised mutexes => %s"), W2CT(sharedKey.c_str())); - auto resource_name = (m_namespace + _T("\\OpenCover_Profiler_Communication_SendData_Event_") + sharedKey); - m_eventProfilerRequestsInformation.Initialise(resource_name.c_str()); - if (!m_eventProfilerRequestsInformation.IsValid()) { + auto resource_name = (_namespace + _T("\\OpenCover_Profiler_Communication_SendData_Event_") + sharedKey); + _eventProfilerRequestsInformation.Initialise(resource_name.c_str()); + if (!_eventProfilerRequestsInformation.IsValid()) { RELTRACE(_T("ProfilerCommunication::Initialise(...) => Failed to initialise resource %s => ::GetLastError() = %d"), W2CT(resource_name.c_str()), ::GetLastError()); return false; } - resource_name = (m_namespace + _T("\\OpenCover_Profiler_Communication_ChunkData_Event_") + sharedKey); - m_eventInformationReadByProfiler.Initialise(resource_name.c_str()); - if (!m_eventInformationReadByProfiler.IsValid()) { + resource_name = (_namespace + _T("\\OpenCover_Profiler_Communication_ChunkData_Event_") + sharedKey); + _eventInformationReadByProfiler.Initialise(resource_name.c_str()); + if (!_eventInformationReadByProfiler.IsValid()) { RELTRACE(_T("ProfilerCommunication::Initialise(...) = >Failed to initialise resource %s => ::GetLastError() = %d"), W2CT(resource_name.c_str()), ::GetLastError()); return false; } - resource_name = (m_namespace + _T("\\OpenCover_Profiler_Communication_ReceiveData_Event_") + sharedKey); - m_eventInformationReadyForProfiler.Initialise(resource_name.c_str()); - if (!m_eventInformationReadyForProfiler.IsValid()) { + resource_name = (_namespace + _T("\\OpenCover_Profiler_Communication_ReceiveData_Event_") + sharedKey); + _eventInformationReadyForProfiler.Initialise(resource_name.c_str()); + if (!_eventInformationReadyForProfiler.IsValid()) { RELTRACE(_T("ProfilerCommunication::Initialise(...) => Failed to initialise resource %s => ::GetLastError() = %d"), W2CT(resource_name.c_str()), ::GetLastError()); return false; } - resource_name = (m_namespace + _T("\\OpenCover_Profiler_Communication_MemoryMapFile_") + sharedKey); - m_memoryCommunication.OpenFileMapping(resource_name.c_str()); - if (!m_memoryCommunication.IsValid()) { + resource_name = (_namespace + _T("\\OpenCover_Profiler_Communication_MemoryMapFile_") + sharedKey); + _memoryCommunication.OpenFileMapping(resource_name.c_str()); + if (!_memoryCommunication.IsValid()) { RELTRACE(_T("ProfilerCommunication::Initialise(...) => Failed to initialise resource %s => ::GetLastError() = %d"), W2CT(resource_name.c_str()), ::GetLastError()); return false; } - resource_name = (m_namespace + _T("\\OpenCover_Profiler_Communication_Semaphore_") + sharedKey); + resource_name = (_namespace + _T("\\OpenCover_Profiler_Communication_Semaphore_") + sharedKey); _semapore_communication.Initialise(resource_name.c_str()); if (!_semapore_communication.IsValid()) { RELTRACE(_T("ProfilerCommunication::Initialise(...) => Failed to initialise resource %s => ::GetLastError() = %d"), W2CT(resource_name.c_str()), ::GetLastError()); return false; } - m_pMSG = (MSG_Union*)m_memoryCommunication.MapViewOfFile(0, 0, MAX_MSG_SIZE); + _pMSG = static_cast(_memoryCommunication.MapViewOfFile(0, 0, MAX_MSG_SIZE)); - hostCommunicationActive = true; + _hostCommunicationActive = true; ATLTRACE(_T("ProfilerCommunication::Initialise(...) => Initialised communication interface => %s"), W2CT(sharedKey.c_str())); @@ -98,104 +98,104 @@ bool ProfilerCommunication::Initialise(TCHAR *key, TCHAR *ns, TCHAR *processName stream << bufferId; stream >> memoryKey; - m_bufferId = bufferId; + _bufferId = bufferId; - memoryKey = m_key + memoryKey; + memoryKey = _key + memoryKey; ATLTRACE(_T("ProfilerCommunication::Initialise(...) => Re-initialising communication interface => %s"), W2CT(memoryKey.c_str())); - resource_name = (m_namespace + _T("\\OpenCover_Profiler_Communication_SendData_Event_") + memoryKey); - m_eventProfilerRequestsInformation.Initialise(resource_name.c_str()); - if (!m_eventProfilerRequestsInformation.IsValid()) { + resource_name = (_namespace + _T("\\OpenCover_Profiler_Communication_SendData_Event_") + memoryKey); + _eventProfilerRequestsInformation.Initialise(resource_name.c_str()); + if (!_eventProfilerRequestsInformation.IsValid()) { RELTRACE(_T("ProfilerCommunication::Initialise(...) => Failed to initialise resource %s => ::GetLastError() = %d"), W2CT(resource_name.c_str()), ::GetLastError()); - hostCommunicationActive = false; + _hostCommunicationActive = false; return false; } - resource_name = (m_namespace + _T("\\OpenCover_Profiler_Communication_ChunkData_Event_") + memoryKey); - m_eventInformationReadByProfiler.Initialise(resource_name.c_str()); - if (!m_eventInformationReadByProfiler.IsValid()) { + resource_name = (_namespace + _T("\\OpenCover_Profiler_Communication_ChunkData_Event_") + memoryKey); + _eventInformationReadByProfiler.Initialise(resource_name.c_str()); + if (!_eventInformationReadByProfiler.IsValid()) { RELTRACE(_T("ProfilerCommunication::Initialise(...) => Failed to initialise resource %s => ::GetLastError() = %d"), W2CT(resource_name.c_str()), ::GetLastError()); - hostCommunicationActive = false; + _hostCommunicationActive = false; return false; } - resource_name = (m_namespace + _T("\\OpenCover_Profiler_Communication_ReceiveData_Event_") + memoryKey); - m_eventInformationReadyForProfiler.Initialise(resource_name.c_str()); - if (!m_eventInformationReadyForProfiler.IsValid()) { + resource_name = (_namespace + _T("\\OpenCover_Profiler_Communication_ReceiveData_Event_") + memoryKey); + _eventInformationReadyForProfiler.Initialise(resource_name.c_str()); + if (!_eventInformationReadyForProfiler.IsValid()) { RELTRACE(_T("ProfilerCommunication::Initialise(...) => Failed to initialise resource %s => ::GetLastError() = %d"), W2CT(resource_name.c_str()), ::GetLastError()); - hostCommunicationActive = false; + _hostCommunicationActive = false; return false; } - resource_name = (m_namespace + _T("\\OpenCover_Profiler_Communication_MemoryMapFile_") + memoryKey); - m_memoryCommunication.OpenFileMapping(resource_name.c_str()); - if (!m_memoryCommunication.IsValid()) { + resource_name = (_namespace + _T("\\OpenCover_Profiler_Communication_MemoryMapFile_") + memoryKey); + _memoryCommunication.OpenFileMapping(resource_name.c_str()); + if (!_memoryCommunication.IsValid()) { RELTRACE(_T("ProfilerCommunication::Initialise(...) => Failed to initialise resource %s => ::GetLastError() = %d"), W2CT(resource_name.c_str()), ::GetLastError()); - hostCommunicationActive = false; + _hostCommunicationActive = false; return false; } - m_pMSG = (MSG_Union*)m_memoryCommunication.MapViewOfFile(0, 0, MAX_MSG_SIZE); + _pMSG = static_cast(_memoryCommunication.MapViewOfFile(0, 0, MAX_MSG_SIZE)); - resource_name = (m_namespace + _T("\\OpenCover_Profiler_Communication_Semaphore_") + memoryKey); + resource_name = (_namespace + _T("\\OpenCover_Profiler_Communication_Semaphore_") + memoryKey); _semapore_communication.Initialise(resource_name.c_str()); if (!_semapore_communication.IsValid()) { RELTRACE(_T("ProfilerCommunication::Initialise(...) => Failed to initialise resource %s => ::GetLastError() = %d"), W2CT(resource_name.c_str()), ::GetLastError()); - hostCommunicationActive = false; + _hostCommunicationActive = false; return false; } ATLTRACE(_T("ProfilerCommunication::Initialise(...) => Re-initialised communication interface => %s"), W2CT(memoryKey.c_str())); - resource_name = (m_namespace + _T("\\OpenCover_Profiler_Results_SendResults_Event_") + memoryKey); - m_eventProfilerHasResults.Initialise(resource_name.c_str()); - if (!m_eventProfilerHasResults.IsValid()) { + resource_name = (_namespace + _T("\\OpenCover_Profiler_Results_SendResults_Event_") + memoryKey); + _eventProfilerHasResults.Initialise(resource_name.c_str()); + if (!_eventProfilerHasResults.IsValid()) { RELTRACE(_T("ProfilerCommunication::Initialise(...) => Failed to initialise resource %s => ::GetLastError() = %d"), W2CT(resource_name.c_str()), ::GetLastError()); - hostCommunicationActive = false; + _hostCommunicationActive = false; return false; } - resource_name = (m_namespace + _T("\\OpenCover_Profiler_Results_ReceiveResults_Event_") + memoryKey); - m_eventResultsHaveBeenReceived.Initialise(resource_name.c_str()); - if (!m_eventResultsHaveBeenReceived.IsValid()) { + resource_name = (_namespace + _T("\\OpenCover_Profiler_Results_ReceiveResults_Event_") + memoryKey); + _eventResultsHaveBeenReceived.Initialise(resource_name.c_str()); + if (!_eventResultsHaveBeenReceived.IsValid()) { RELTRACE(_T("ProfilerCommunication::Initialise(...) => Failed to initialise resource %s => ::GetLastError() = %d"), W2CT(resource_name.c_str()), ::GetLastError()); - hostCommunicationActive = false; + _hostCommunicationActive = false; return false; } - resource_name = (m_namespace + _T("\\OpenCover_Profiler_Results_MemoryMapFile_") + memoryKey); - m_memoryResults.OpenFileMapping(resource_name.c_str()); - if (!m_memoryResults.IsValid()) { + resource_name = (_namespace + _T("\\OpenCover_Profiler_Results_MemoryMapFile_") + memoryKey); + _memoryResults.OpenFileMapping(resource_name.c_str()); + if (!_memoryResults.IsValid()) { RELTRACE(_T("ProfilerCommunication::Initialise(...) => Failed to initialise resource %s => ::GetLastError() = %d"), W2CT(resource_name.c_str()), ::GetLastError()); - hostCommunicationActive = false; + _hostCommunicationActive = false; return false; } - m_pVisitPoints = (MSG_SendVisitPoints_Request*)m_memoryResults.MapViewOfFile(0, 0, MAX_MSG_SIZE); + _pVisitPoints = static_cast(_memoryResults.MapViewOfFile(0, 0, MAX_MSG_SIZE)); - m_pVisitPoints->count = 0; + _pVisitPoints->count = 0; - resource_name = (m_namespace + _T("\\OpenCover_Profiler_Results_Semaphore_") + memoryKey); + resource_name = (_namespace + _T("\\OpenCover_Profiler_Results_Semaphore_") + memoryKey); _semapore_results.Initialise(resource_name.c_str()); if (!_semapore_results.IsValid()) { RELTRACE(_T("ProfilerCommunication::Initialise(...) => Failed to initialise resource %s => ::GetLastError() = %d"), W2CT(resource_name.c_str()), ::GetLastError()); - hostCommunicationActive = false; + _hostCommunicationActive = false; return false; } RELTRACE(_T("ProfilerCommunication::Initialise(...) => Initialised results interface => %s"), W2CT(memoryKey.c_str())); } else { - hostCommunicationActive = false; + _hostCommunicationActive = false; } - return hostCommunicationActive; + return _hostCommunicationActive; } void ProfilerCommunication::ThreadCreated(ThreadID threadID, DWORD osThreadID){ - ATL::CComCritSecLock lock(m_critThreads); - m_threadmap[threadID] = osThreadID; + ATL::CComCritSecLock lock(_critThreads); + _threadmap[threadID] = osThreadID; AllocateVisitMap(osThreadID); } @@ -203,14 +203,14 @@ MSG_SendVisitPoints_Request* ProfilerCommunication::AllocateVisitMap(DWORD osThr auto p = new MSG_SendVisitPoints_Request(); p->count = 0; //::ZeroMemory(p, sizeof(MSG_SendVisitPoints_Request)); - m_visitmap[osThreadID] = p; + _visitmap[osThreadID] = p; return p; } MSG_SendVisitPoints_Request* ProfilerCommunication::GetVisitMapForOSThread(ULONG osThreadID){ MSG_SendVisitPoints_Request * p; try { - p = m_visitmap[osThreadID]; + p = _visitmap[osThreadID]; if (p == nullptr) p = AllocateVisitMap(osThreadID); } @@ -221,18 +221,18 @@ MSG_SendVisitPoints_Request* ProfilerCommunication::GetVisitMapForOSThread(ULONG } void ProfilerCommunication::ThreadDestroyed(ThreadID threadID){ - ATL::CComCritSecLock lock(m_critThreads); - ULONG osThreadId = m_threadmap[threadID]; - auto points = m_visitmap[osThreadId]; + ATL::CComCritSecLock lock(_critThreads); + ULONG osThreadId = _threadmap[threadID]; + auto points = _visitmap[osThreadId]; SendThreadVisitPoints(points); - delete m_visitmap[osThreadId]; - m_visitmap[osThreadId] = NULL; + delete _visitmap[osThreadId]; + _visitmap[osThreadId] = nullptr; } void ProfilerCommunication::SendRemainingThreadBuffers(){ - ATL::CComCritSecLock lock(m_critThreads); - for (auto it = m_visitmap.begin(); it != m_visitmap.end(); ++it){ - if (it->second != NULL){ + ATL::CComCritSecLock lock(_critThreads); + for (auto it = _visitmap.begin(); it != _visitmap.end(); ++it){ + if (it->second != nullptr){ SendThreadVisitPoints(it->second); //::ZeroMemory(pVisitPoints, sizeof(MSG_SendVisitPoints_Request)); } @@ -252,62 +252,62 @@ void ProfilerCommunication::AddVisitPointToThreadBuffer(ULONG uniqueId, MSG_IdTy } void ProfilerCommunication::SendThreadVisitPoints(MSG_SendVisitPoints_Request* pVisitPoints){ - ATL::CComCritSecLock lock(m_critResults); + ATL::CComCritSecLock lock(_critResults); - if (!hostCommunicationActive) + if (!_hostCommunicationActive) return; if (!TestSemaphore(_semapore_results)) return; handle_exception([=](){ - memcpy(m_pVisitPoints, pVisitPoints, sizeof(MSG_SendVisitPoints_Request)); + memcpy(_pVisitPoints, pVisitPoints, sizeof(MSG_SendVisitPoints_Request)); }, _T("SendThreadVisitPoints")); pVisitPoints->count = 0; SendVisitPoints(); - //::ZeroMemory(m_pVisitPoints, sizeof(MSG_SendVisitPoints_Request)); - m_pVisitPoints->count = 0; + //::ZeroMemory(_pVisitPoints, sizeof(MSG_SendVisitPoints_Request)); + _pVisitPoints->count = 0; } void ProfilerCommunication::AddVisitPointToBuffer(ULONG uniqueId, MSG_IdType msgType) { - ATL::CComCritSecLock lock(m_critResults); + ATL::CComCritSecLock lock(_critResults); - if (!hostCommunicationActive) + if (!_hostCommunicationActive) return; if (!TestSemaphore(_semapore_results)) return; handle_exception([=](){ - m_pVisitPoints->points[m_pVisitPoints->count].UniqueId = (uniqueId | msgType); + _pVisitPoints->points[_pVisitPoints->count].UniqueId = (uniqueId | msgType); }, _T("AddVisitPointToBuffer")); - if (++m_pVisitPoints->count == VP_BUFFER_SIZE) + if (++_pVisitPoints->count == VP_BUFFER_SIZE) { SendVisitPoints(); - //::ZeroMemory(m_pVisitPoints, sizeof(MSG_SendVisitPoints_Request)); + //::ZeroMemory(_pVisitPoints, sizeof(MSG_SendVisitPoints_Request)); handle_exception([=](){ - m_pVisitPoints->count = 0; + _pVisitPoints->count = 0; }, _T("AddVisitPointToBuffer")); } } void ProfilerCommunication::SendVisitPoints() { - if (!hostCommunicationActive) + if (!_hostCommunicationActive) return; try { - m_memoryResults.FlushViewOfFile(); + _memoryResults.FlushViewOfFile(); - DWORD dwSignal = m_eventProfilerHasResults.SignalAndWait(m_eventResultsHaveBeenReceived, COMM_WAIT_SHORT); - if (WAIT_OBJECT_0 != dwSignal) throw CommunicationException(dwSignal, COMM_WAIT_SHORT); - m_eventResultsHaveBeenReceived.Reset(); + DWORD dwSignal = _eventProfilerHasResults.SignalAndWait(_eventResultsHaveBeenReceived, _short_wait); + if (WAIT_OBJECT_0 != dwSignal) throw CommunicationException(dwSignal, _short_wait); + _eventResultsHaveBeenReceived.Reset(); } catch (CommunicationException ex) { RELTRACE(_T("ProfilerCommunication::SendVisitPoints() => Communication (Results channel) with host has failed (0x%x, %d)"), ex.getReason(), ex.getTimeout()); - hostCommunicationActive = false; + _hostCommunicationActive = false; } return; } @@ -329,35 +329,35 @@ bool ProfilerCommunication::GetPoints(mdToken functionToken, WCHAR* pModulePath, bool ProfilerCommunication::GetSequencePoints(mdToken functionToken, WCHAR* pModulePath, WCHAR* pAssemblyName, std::vector &points) { - if (!hostCommunicationActive) + if (!_hostCommunicationActive) return false; RequestInformation( [=] { - m_pMSG->getSequencePointsRequest.type = MSG_GetSequencePoints; - m_pMSG->getSequencePointsRequest.functionToken = functionToken; + _pMSG->getSequencePointsRequest.type = MSG_GetSequencePoints; + _pMSG->getSequencePointsRequest.functionToken = functionToken; USES_CONVERSION; - wcscpy_s(m_pMSG->getSequencePointsRequest.szProcessName, T2CW(m_processName.c_str())); - wcscpy_s(m_pMSG->getSequencePointsRequest.szModulePath, pModulePath); - wcscpy_s(m_pMSG->getSequencePointsRequest.szAssemblyName, pAssemblyName); + wcscpy_s(_pMSG->getSequencePointsRequest.szProcessName, T2CW(_processName.c_str())); + wcscpy_s(_pMSG->getSequencePointsRequest.szModulePath, pModulePath); + wcscpy_s(_pMSG->getSequencePointsRequest.szAssemblyName, pAssemblyName); }, [=, &points]()->BOOL { - if (m_pMSG->getSequencePointsResponse.count > SEQ_BUFFER_SIZE){ + if (_pMSG->getSequencePointsResponse.count > SEQ_BUFFER_SIZE){ RELTRACE(_T("Received an abnormal count for sequence points (%d) for token 0x%X"), - m_pMSG->getSequencePointsResponse.count, functionToken); + _pMSG->getSequencePointsResponse.count, functionToken); points.clear(); return false; } - for (int i = 0; i < m_pMSG->getSequencePointsResponse.count; i++) - points.push_back(m_pMSG->getSequencePointsResponse.points[i]); - BOOL hasMore = m_pMSG->getSequencePointsResponse.hasMore; - ::ZeroMemory(m_pMSG, MAX_MSG_SIZE); + for (int i = 0; i < _pMSG->getSequencePointsResponse.count; i++) + points.push_back(_pMSG->getSequencePointsResponse.points[i]); + BOOL hasMore = _pMSG->getSequencePointsResponse.hasMore; + ::ZeroMemory(_pMSG, MAX_MSG_SIZE); return hasMore; } - , COMM_WAIT_SHORT + , _short_wait , _T("GetSequencePoints")); return (points.size() != 0); @@ -366,35 +366,35 @@ bool ProfilerCommunication::GetSequencePoints(mdToken functionToken, WCHAR* pMod bool ProfilerCommunication::GetBranchPoints(mdToken functionToken, WCHAR* pModulePath, WCHAR* pAssemblyName, std::vector &points) { - if (!hostCommunicationActive) + if (!_hostCommunicationActive) return false; RequestInformation( [=] { - m_pMSG->getBranchPointsRequest.type = MSG_GetBranchPoints; - m_pMSG->getBranchPointsRequest.functionToken = functionToken; + _pMSG->getBranchPointsRequest.type = MSG_GetBranchPoints; + _pMSG->getBranchPointsRequest.functionToken = functionToken; USES_CONVERSION; - wcscpy_s(m_pMSG->getBranchPointsRequest.szProcessName, T2CW(m_processName.c_str())); - wcscpy_s(m_pMSG->getBranchPointsRequest.szModulePath, pModulePath); - wcscpy_s(m_pMSG->getBranchPointsRequest.szAssemblyName, pAssemblyName); + wcscpy_s(_pMSG->getBranchPointsRequest.szProcessName, T2CW(_processName.c_str())); + wcscpy_s(_pMSG->getBranchPointsRequest.szModulePath, pModulePath); + wcscpy_s(_pMSG->getBranchPointsRequest.szAssemblyName, pAssemblyName); }, [=, &points]()->BOOL { - if (m_pMSG->getBranchPointsResponse.count > BRANCH_BUFFER_SIZE){ + if (_pMSG->getBranchPointsResponse.count > BRANCH_BUFFER_SIZE){ RELTRACE(_T("Received an abnormal count for branch points (%d) for token 0x%X"), - m_pMSG->getBranchPointsResponse.count, functionToken); + _pMSG->getBranchPointsResponse.count, functionToken); points.clear(); return false; } - for (int i=0; i < m_pMSG->getBranchPointsResponse.count;i++) - points.push_back(m_pMSG->getBranchPointsResponse.points[i]); - BOOL hasMore = m_pMSG->getBranchPointsResponse.hasMore; - ::ZeroMemory(m_pMSG, MAX_MSG_SIZE); + for (int i=0; i < _pMSG->getBranchPointsResponse.count;i++) + points.push_back(_pMSG->getBranchPointsResponse.points[i]); + BOOL hasMore = _pMSG->getBranchPointsResponse.hasMore; + ::ZeroMemory(_pMSG, MAX_MSG_SIZE); return hasMore; } - , COMM_WAIT_SHORT + , _short_wait , _T("GetBranchPoints")); return (points.size() != 0); @@ -402,26 +402,26 @@ bool ProfilerCommunication::GetBranchPoints(mdToken functionToken, WCHAR* pModul bool ProfilerCommunication::TrackAssembly(WCHAR* pModulePath, WCHAR* pAssemblyName) { - if (!hostCommunicationActive) + if (!_hostCommunicationActive) return false; bool response = false; RequestInformation( [=]() { - m_pMSG->trackAssemblyRequest.type = MSG_TrackAssembly; + _pMSG->trackAssemblyRequest.type = MSG_TrackAssembly; USES_CONVERSION; - wcscpy_s(m_pMSG->trackAssemblyRequest.szProcessName, T2CW(m_processName.c_str())); - wcscpy_s(m_pMSG->trackAssemblyRequest.szModulePath, pModulePath); - wcscpy_s(m_pMSG->trackAssemblyRequest.szAssemblyName, pAssemblyName); + wcscpy_s(_pMSG->trackAssemblyRequest.szProcessName, T2CW(_processName.c_str())); + wcscpy_s(_pMSG->trackAssemblyRequest.szModulePath, pModulePath); + wcscpy_s(_pMSG->trackAssemblyRequest.szAssemblyName, pAssemblyName); }, [=, &response]()->BOOL { - response = m_pMSG->trackAssemblyResponse.bResponse == TRUE; - ::ZeroMemory(m_pMSG, MAX_MSG_SIZE); + response = _pMSG->trackAssemblyResponse.bResponse == TRUE; + ::ZeroMemory(_pMSG, MAX_MSG_SIZE); return FALSE; } - , COMM_WAIT_LONG + , COM_WAIT_LONG , _T("TrackAssembly")); return response; @@ -429,26 +429,26 @@ bool ProfilerCommunication::TrackAssembly(WCHAR* pModulePath, WCHAR* pAssemblyNa bool ProfilerCommunication::TrackMethod(mdToken functionToken, WCHAR* pModulePath, WCHAR* pAssemblyName, ULONG &uniqueId) { - if (!hostCommunicationActive) + if (!_hostCommunicationActive) return false; bool response = false; RequestInformation( [=]() { - m_pMSG->trackMethodRequest.type = MSG_TrackMethod; - m_pMSG->trackMethodRequest.functionToken = functionToken; - wcscpy_s(m_pMSG->trackMethodRequest.szModulePath, pModulePath); - wcscpy_s(m_pMSG->trackMethodRequest.szAssemblyName, pAssemblyName); + _pMSG->trackMethodRequest.type = MSG_TrackMethod; + _pMSG->trackMethodRequest.functionToken = functionToken; + wcscpy_s(_pMSG->trackMethodRequest.szModulePath, pModulePath); + wcscpy_s(_pMSG->trackMethodRequest.szAssemblyName, pAssemblyName); }, [=, &response, &uniqueId]()->BOOL { - response = m_pMSG->trackMethodResponse.bResponse == TRUE; - uniqueId = m_pMSG->trackMethodResponse.ulUniqueId; - ::ZeroMemory(m_pMSG, MAX_MSG_SIZE); + response = _pMSG->trackMethodResponse.bResponse == TRUE; + uniqueId = _pMSG->trackMethodResponse.ulUniqueId; + ::ZeroMemory(_pMSG, MAX_MSG_SIZE); return FALSE; } - , COMM_WAIT_SHORT + , _short_wait , _T("TrackMethod")); return response; @@ -456,29 +456,29 @@ bool ProfilerCommunication::TrackMethod(mdToken functionToken, WCHAR* pModulePat bool ProfilerCommunication::AllocateBuffer(LONG bufferSize, ULONG &bufferId) { - CScopedLock lock(m_mutexCommunication); + CScopedLock lock(_mutexCommunication); - if (!hostCommunicationActive) + if (!_hostCommunicationActive) return false; bool response = false; int repeat = 0; while (!response && (++repeat <= 3)){ - hostCommunicationActive = true; + _hostCommunicationActive = true; RequestInformation( [=]() { - m_pMSG->allocateBufferRequest.type = MSG_AllocateMemoryBuffer; - m_pMSG->allocateBufferRequest.lBufferSize = bufferSize; + _pMSG->allocateBufferRequest.type = MSG_AllocateMemoryBuffer; + _pMSG->allocateBufferRequest.lBufferSize = bufferSize; }, [=, &response, &bufferId]()->BOOL { - response = m_pMSG->allocateBufferResponse.bResponse == TRUE; - bufferId = m_pMSG->allocateBufferResponse.ulBufferId; - ::ZeroMemory(m_pMSG, MAX_MSG_SIZE); + response = _pMSG->allocateBufferResponse.bResponse == TRUE; + bufferId = _pMSG->allocateBufferResponse.ulBufferId; + ::ZeroMemory(_pMSG, MAX_MSG_SIZE); return FALSE; } - , COMM_WAIT_VSHORT + , COM_WAIT_VSHORT , _T("AllocateBuffer")); } @@ -486,10 +486,10 @@ bool ProfilerCommunication::AllocateBuffer(LONG bufferSize, ULONG &bufferId) } void ProfilerCommunication::CloseChannel(bool sendSingleBuffer){ - if (m_bufferId == 0) + if (_bufferId == 0) return; - if (!hostCommunicationActive) + if (!_hostCommunicationActive) return; if (!TestSemaphore(_semapore_results)) @@ -500,7 +500,7 @@ void ProfilerCommunication::CloseChannel(bool sendSingleBuffer){ else SendRemainingThreadBuffers(); - if (!hostCommunicationActive) + if (!_hostCommunicationActive) return; bool response = false; @@ -508,24 +508,24 @@ void ProfilerCommunication::CloseChannel(bool sendSingleBuffer){ RequestInformation( [=]() { - m_pMSG->closeChannelRequest.type = MSG_CloseChannel; - m_pMSG->closeChannelRequest.ulBufferId = m_bufferId; + _pMSG->closeChannelRequest.type = MSG_CloseChannel; + _pMSG->closeChannelRequest.ulBufferId = _bufferId; }, [=, &response]()->BOOL { - response = m_pMSG->allocateBufferResponse.bResponse == TRUE; + response = _pMSG->allocateBufferResponse.bResponse == TRUE; return FALSE; } - , COMM_WAIT_SHORT + , _short_wait , _T("CloseChannel")); return; } bool ProfilerCommunication::TrackProcess(){ - CScopedLock lock(m_mutexCommunication); + CScopedLock lock(_mutexCommunication); - if (!hostCommunicationActive) + if (!_hostCommunicationActive) return false; bool response = false; @@ -533,16 +533,16 @@ bool ProfilerCommunication::TrackProcess(){ RequestInformation( [=]() { - m_pMSG->trackProcessRequest.type = MSG_TrackProcess; + _pMSG->trackProcessRequest.type = MSG_TrackProcess; USES_CONVERSION; - wcscpy_s(m_pMSG->trackProcessRequest.szProcessName, T2CW(m_processName.c_str())); + wcscpy_s(_pMSG->trackProcessRequest.szProcessName, T2CW(_processName.c_str())); }, [=, &response]()->BOOL { - response = m_pMSG->trackProcessResponse.bResponse == TRUE; + response = _pMSG->trackProcessResponse.bResponse == TRUE; return FALSE; } - , COMM_WAIT_SHORT + , _short_wait , _T("TrackProcess")); return response; @@ -602,8 +602,8 @@ void ProfilerCommunication::handle_exception(Action action, const tstring& messa template void ProfilerCommunication::RequestInformation(BR buildRequest, PR processResults, DWORD dwTimeout, tstring message) { - ATL::CComCritSecLock lock(m_critComms); - if (!hostCommunicationActive) + ATL::CComCritSecLock lock(_critComms); + if (!_hostCommunicationActive) return; if (!TestSemaphore(_semapore_communication)) @@ -613,12 +613,12 @@ void ProfilerCommunication::RequestInformation(BR buildRequest, PR processResult handle_exception([&](){ buildRequest(); }, message); - m_memoryCommunication.FlushViewOfFile(); + _memoryCommunication.FlushViewOfFile(); - DWORD dwSignal = m_eventProfilerRequestsInformation.SignalAndWait(m_eventInformationReadyForProfiler, dwTimeout); + DWORD dwSignal = _eventProfilerRequestsInformation.SignalAndWait(_eventInformationReadyForProfiler, dwTimeout); if (WAIT_OBJECT_0 != dwSignal) throw CommunicationException(dwSignal, dwTimeout); - m_eventInformationReadyForProfiler.Reset(); + _eventInformationReadyForProfiler.Reset(); BOOL hasMore = FALSE; do @@ -627,21 +627,21 @@ void ProfilerCommunication::RequestInformation(BR buildRequest, PR processResult if (hasMore) { - dwSignal = m_eventInformationReadByProfiler.SignalAndWait(m_eventInformationReadyForProfiler, COMM_WAIT_SHORT); - if (WAIT_OBJECT_0 != dwSignal) throw CommunicationException(dwSignal, COMM_WAIT_SHORT); + dwSignal = _eventInformationReadByProfiler.SignalAndWait(_eventInformationReadyForProfiler, _short_wait); + if (WAIT_OBJECT_0 != dwSignal) throw CommunicationException(dwSignal, _short_wait); - m_eventInformationReadyForProfiler.Reset(); + _eventInformationReadyForProfiler.Reset(); } }while (hasMore); - m_eventInformationReadByProfiler.Set(); + _eventInformationReadByProfiler.Set(); } catch (CommunicationException ex) { RELTRACE(_T("ProfilerCommunication::RequestInformation(...) => Communication (Chat channel - %s) with host has failed (0x%x, %d)"), message.c_str(), ex.getReason(), ex.getTimeout()); - hostCommunicationActive = false; + _hostCommunicationActive = false; } catch (...) { - hostCommunicationActive = false; + _hostCommunicationActive = false; } } diff --git a/main/OpenCover.Profiler/ProfilerCommunication.h b/main/OpenCover.Profiler/ProfilerCommunication.h index 0d5e0518f..c07341d29 100644 --- a/main/OpenCover.Profiler/ProfilerCommunication.h +++ b/main/OpenCover.Profiler/ProfilerCommunication.h @@ -20,7 +20,7 @@ class ProfilerCommunication private: public: - ProfilerCommunication(); + ProfilerCommunication(DWORD short_wait); ~ProfilerCommunication(void); bool Initialise(TCHAR* key, TCHAR *ns, TCHAR *processName); @@ -53,49 +53,50 @@ class ProfilerCommunication MSG_SendVisitPoints_Request* AllocateVisitMap(DWORD osThreadID); private: - tstring m_key; - tstring m_namespace; - tstring m_processName; + tstring _key; + tstring _namespace; + tstring _processName; + DWORD _short_wait; template void RequestInformation(BR buildRequest, PR processResults, DWORD dwTimeout, tstring message); - ULONG m_bufferId; + ULONG _bufferId; bool TestSemaphore(CSemaphoreEx &semaphore){ // the previous value should always be zero unless the host process has released // and that means we have disposed of the shared memory - if (hostCommunicationActive && semaphore.ReleaseAndWait() != 0) { - hostCommunicationActive = false; + if (_hostCommunicationActive && semaphore.ReleaseAndWait() != 0) { + _hostCommunicationActive = false; } - return hostCommunicationActive; + return _hostCommunicationActive; } private: - CMutex m_mutexCommunication; - CSharedMemory m_memoryCommunication; - CEvent m_eventProfilerRequestsInformation; - CEvent m_eventInformationReadyForProfiler; - CEvent m_eventInformationReadByProfiler; - MSG_Union *m_pMSG; + CMutex _mutexCommunication; + CSharedMemory _memoryCommunication; + CEvent _eventProfilerRequestsInformation; + CEvent _eventInformationReadyForProfiler; + CEvent _eventInformationReadByProfiler; + MSG_Union *_pMSG; CSemaphoreEx _semapore_communication; private: - CSharedMemory m_memoryResults; - CEvent m_eventProfilerHasResults; - CEvent m_eventResultsHaveBeenReceived; - MSG_SendVisitPoints_Request *m_pVisitPoints; + CSharedMemory _memoryResults; + CEvent _eventProfilerHasResults; + CEvent _eventResultsHaveBeenReceived; + MSG_SendVisitPoints_Request *_pVisitPoints; CSemaphoreEx _semapore_results; private: - ATL::CComAutoCriticalSection m_critResults; - ATL::CComAutoCriticalSection m_critComms; - bool hostCommunicationActive; + ATL::CComAutoCriticalSection _critResults; + ATL::CComAutoCriticalSection _critComms; + bool _hostCommunicationActive; private: - ATL::CComAutoCriticalSection m_critThreads; - std::unordered_map m_threadmap; - std::unordered_map m_visitmap; + ATL::CComAutoCriticalSection _critThreads; + std::unordered_map _threadmap; + std::unordered_map _visitmap; MSG_SendVisitPoints_Request* GetVisitMapForOSThread(ULONG osThread); From 59c8534a11c4ee38c1fb6c7a40cee9bc327b16a0 Mon Sep 17 00:00:00 2001 From: sawilde Date: Wed, 2 Dec 2015 07:55:54 +1100 Subject: [PATCH 04/10] #335 extract communication timeout value --- main/OpenCover.Framework/CommandLineParser.cs | 10 ++++++ .../Framework/CommandLineParserTests.cs | 36 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/main/OpenCover.Framework/CommandLineParser.cs b/main/OpenCover.Framework/CommandLineParser.cs index a6d01570f..339cffc0a 100644 --- a/main/OpenCover.Framework/CommandLineParser.cs +++ b/main/OpenCover.Framework/CommandLineParser.cs @@ -190,6 +190,11 @@ public void ExtractAndValidateArguments() ReturnCodeOffset = ExtractValue("returntargetcode", () => { throw new InvalidOperationException("The return target code offset must be an integer"); }); break; + case "communicationtimeout": + CommunicationTimeout = ExtractValue("communicationtimeout", () => + { throw new InvalidOperationException(string.Format("The communicationtimeout must be an integer: {0}", GetArgumentValue("communicationtimeout"))); }); + CommunicationTimeout = Math.Max(Math.Min(CommunicationTimeout, 60000), 10000); + break; case "filter": Filters = ExtractFilters(GetArgumentValue("filter")); break; @@ -511,6 +516,11 @@ private void ValidateArguments() /// Instructs the console to print its version and exit /// public bool PrintVersion { get; private set; } + + /// + /// Sets the 'short' timeout between profiler and host (normally 10000ms) + /// + public int CommunicationTimeout { get; private set; } } } \ No newline at end of file diff --git a/main/OpenCover.Test/Framework/CommandLineParserTests.cs b/main/OpenCover.Test/Framework/CommandLineParserTests.cs index 1f9deaf4a..ec07cffe9 100644 --- a/main/OpenCover.Test/Framework/CommandLineParserTests.cs +++ b/main/OpenCover.Test/Framework/CommandLineParserTests.cs @@ -719,6 +719,7 @@ public void HandlesServiceStartTimeout(string timeAsString, int expectedMinutes, Assert.That(parser.ServiceStartTimeout, Is.EqualTo(new TimeSpan(0, expectedMinutes, expectedSeconds))); } + [Test] [TestCase("10")] [TestCase("NaNs")] [TestCase("indifferenttext")] @@ -735,6 +736,41 @@ public void InvalidServiceStartTimeoutThrowsException(string invalidTimeout) Assert.That(thrownException.Message, Contains.Substring(invalidTimeout)); } + [Test] + [TestCase(10000, 10000)] + [TestCase(30000, 30000)] + [TestCase(60000, 60000)] + [TestCase(100, 10000)] + [TestCase(70000, 60000)] + public void HandlesCommunicationTimeout(int suppliedMillisconds, int expectedMiliseconds) + { + // arrange + var parser = new CommandLineParser(new[] { string.Format("-communicationtimeout:{0}", suppliedMillisconds), RequiredArgs }); + + // act + parser.ExtractAndValidateArguments(); + + // assert + Assert.That(parser.CommunicationTimeout, Is.EqualTo(expectedMiliseconds)); + + } + + [Test] + [TestCase("NaNs")] + [TestCase("indifferenttext")] + public void InvalidServiceCommunicationTimeoutThrowsException(string invalidTimeout) + { + // arrange + var parser = new CommandLineParser(new[] { "-communicationtimeout:" + invalidTimeout, RequiredArgs }); + + // act + var thrownException = Assert.Throws(parser.ExtractAndValidateArguments); + + // assert + Assert.That(thrownException.Message, Contains.Substring("communicationtimeout")); + Assert.That(thrownException.Message, Contains.Substring(invalidTimeout)); + } + [Test] [TestCase("-{nunit-console*}[*]* -{pdb*}[*]* -{nunit-agent*}[*]*")] [TestCase("-[System*]* -[Xyz*]* -[Zap*]*")] From 96385737c3ec9c16ca358688bbe52e16f7c58477 Mon Sep 17 00:00:00 2001 From: sawilde Date: Wed, 2 Dec 2015 07:56:24 +1100 Subject: [PATCH 05/10] update dogfood.cmd --- main/cmdline/dogfood.cmd | 1 + 1 file changed, 1 insertion(+) diff --git a/main/cmdline/dogfood.cmd b/main/cmdline/dogfood.cmd index 17ec381f5..3a618e264 100644 --- a/main/cmdline/dogfood.cmd +++ b/main/cmdline/dogfood.cmd @@ -1,3 +1,4 @@ +@echo off pushd %cd% cd %~dp0 OpenCover.Console.exe -register:user -target:..\..\..\main\packages\NUnit.Runners.2.6.4\tools\nunit-console-x86.exe -targetargs:"OpenCover.Test.dll /noshadow /exclude:AdminOnly" -filter:"+[Open*]* -[OpenCover.T*]* -{nunit-console*}[*]* -{pdb*}[*]*" -output:opencovertests.xml From c36eb67936702fc954b6bc26c2a625b095d73e63 Mon Sep 17 00:00:00 2001 From: sawilde Date: Tue, 8 Dec 2015 08:37:01 +1100 Subject: [PATCH 06/10] #335 pass connection timeout value through --- main/OpenCover.Framework/CommandLineParser.cs | 5 ++-- main/OpenCover.Framework/ICommandLine.cs | 5 ++++ .../Manager/ProfilerManager.cs | 5 +++- .../Framework/CommandLineParserTests.cs | 2 +- .../Framework/Manager/ProfilerManagerTests.cs | 27 +++++++++++++++++++ main/cmdline/dogfood.cmd | 2 +- 6 files changed, 41 insertions(+), 5 deletions(-) diff --git a/main/OpenCover.Framework/CommandLineParser.cs b/main/OpenCover.Framework/CommandLineParser.cs index 339cffc0a..5fc4d91fb 100644 --- a/main/OpenCover.Framework/CommandLineParser.cs +++ b/main/OpenCover.Framework/CommandLineParser.cs @@ -113,7 +113,8 @@ public string Usage() builder.AppendLine(" [-hideskipped:File|Filter|Attribute|MissingPdb|All,[File|Filter|Attribute|MissingPdb|All]]"); builder.AppendLine(" [-log:[Off|Fatal|Error|Warn|Info|Debug|Verbose|All]]"); builder.AppendLine(" [-service[:byname]]"); - builder.AppendLine(" [-servicestarttimeout:1m23s"); + builder.AppendLine(" [-servicestarttimeout:"); + builder.AppendLine(" [-communicationtimeout:"); builder.AppendLine(" [-threshold:]"); builder.AppendLine(" [-enableperformancecounters]"); builder.AppendLine(" [-skipautoprops]"); @@ -192,7 +193,7 @@ public void ExtractAndValidateArguments() break; case "communicationtimeout": CommunicationTimeout = ExtractValue("communicationtimeout", () => - { throw new InvalidOperationException(string.Format("The communicationtimeout must be an integer: {0}", GetArgumentValue("communicationtimeout"))); }); + { throw new InvalidOperationException(string.Format("The communication timeout must be an integer: {0}", GetArgumentValue("communicationtimeout"))); }); CommunicationTimeout = Math.Max(Math.Min(CommunicationTimeout, 60000), 10000); break; case "filter": diff --git a/main/OpenCover.Framework/ICommandLine.cs b/main/OpenCover.Framework/ICommandLine.cs index 93a295475..eb697c417 100644 --- a/main/OpenCover.Framework/ICommandLine.cs +++ b/main/OpenCover.Framework/ICommandLine.cs @@ -53,5 +53,10 @@ public interface ICommandLine /// Should auto implemented properties be skipped /// bool SkipAutoImplementedProperties { get; } + + /// + /// Sets the 'short' timeout between profiler and host + /// + int CommunicationTimeout { get; } } } \ No newline at end of file diff --git a/main/OpenCover.Framework/Manager/ProfilerManager.cs b/main/OpenCover.Framework/Manager/ProfilerManager.cs index 106ea3113..b0045ee08 100644 --- a/main/OpenCover.Framework/Manager/ProfilerManager.cs +++ b/main/OpenCover.Framework/Manager/ProfilerManager.cs @@ -129,7 +129,10 @@ private void SetProfilerAttributesOnDictionary(string profilerKey, string profil dictionary["CoreClr_Enable_Profiling"] = "1"; dictionary["Cor_Profiler_Path"] = string.Empty; dictionary["CorClr_Profiler_Path"] = string.Empty; - + + if (_commandLine.CommunicationTimeout > 0) + dictionary["OpenCover_Profiler_ShortWait"] = _commandLine.CommunicationTimeout.ToString(); + switch (_commandLine.Registration) { case Registration.Path32: diff --git a/main/OpenCover.Test/Framework/CommandLineParserTests.cs b/main/OpenCover.Test/Framework/CommandLineParserTests.cs index ec07cffe9..bb0361fa1 100644 --- a/main/OpenCover.Test/Framework/CommandLineParserTests.cs +++ b/main/OpenCover.Test/Framework/CommandLineParserTests.cs @@ -767,7 +767,7 @@ public void InvalidServiceCommunicationTimeoutThrowsException(string invalidTime var thrownException = Assert.Throws(parser.ExtractAndValidateArguments); // assert - Assert.That(thrownException.Message, Contains.Substring("communicationtimeout")); + Assert.That(thrownException.Message, Contains.Substring("communication timeout")); Assert.That(thrownException.Message, Contains.Substring(invalidTimeout)); } diff --git a/main/OpenCover.Test/Framework/Manager/ProfilerManagerTests.cs b/main/OpenCover.Test/Framework/Manager/ProfilerManagerTests.cs index e7c593333..709ed20aa 100644 --- a/main/OpenCover.Test/Framework/Manager/ProfilerManagerTests.cs +++ b/main/OpenCover.Test/Framework/Manager/ProfilerManagerTests.cs @@ -77,6 +77,33 @@ public void Manager_Adds_Supplied_Threshold_EnvironmentVariable() Assert.NotNull(dict[@"OpenCover_Profiler_Threshold"]); Assert.AreEqual("500", dict[@"OpenCover_Profiler_Threshold"]); } + [Test] + public void Manager_DoesNotAdd_ShortWait_EnvironmentVariable() + { + // arrange + var dict = new StringDictionary(); + + // act + RunSimpleProcess(dict); + + // assert + Assert.Null(dict[@"OpenCover_Profiler_ShortWait"]); + } + + [Test] + public void Manager_Adds_Supplied_ShortWait_EnvironmentVariable() + { + // arrange + var dict = new StringDictionary(); + Container.GetMock().SetupGet(x => x.CommunicationTimeout).Returns(10000); + + // act + RunSimpleProcess(dict); + + // assert + Assert.NotNull(dict[@"OpenCover_Profiler_ShortWait"]); + Assert.AreEqual("10000", dict[@"OpenCover_Profiler_ShortWait"]); + } [Test] public void Manager_Adds_TraceByTest_EnvironmentVariable_When_Tracing_Enabled() diff --git a/main/cmdline/dogfood.cmd b/main/cmdline/dogfood.cmd index 3a618e264..d33e17758 100644 --- a/main/cmdline/dogfood.cmd +++ b/main/cmdline/dogfood.cmd @@ -1,5 +1,5 @@ @echo off pushd %cd% cd %~dp0 -OpenCover.Console.exe -register:user -target:..\..\..\main\packages\NUnit.Runners.2.6.4\tools\nunit-console-x86.exe -targetargs:"OpenCover.Test.dll /noshadow /exclude:AdminOnly" -filter:"+[Open*]* -[OpenCover.T*]* -{nunit-console*}[*]* -{pdb*}[*]*" -output:opencovertests.xml +OpenCover.Console.exe -register:user -target:..\..\..\main\packages\NUnit.Runners.2.6.4\tools\nunit-console-x86.exe -targetargs:"OpenCover.Test.dll /noshadow /exclude:AdminOnly" -filter:"+[Open*]* -[OpenCover.T*]* -{nunit-console*}[*]* -{pdb*}[*]*" -output:opencovertests.xml -communicationtimeout:9999 popd \ No newline at end of file From 9f2bc789f4897927ffb4902ea15c33106d66b868 Mon Sep 17 00:00:00 2001 From: sawilde Date: Mon, 21 Dec 2015 16:38:06 +0000 Subject: [PATCH 07/10] #376 use concurrent map --- .../ProfilerCommunication.cpp | 32 +++++----- .../ProfilerCommunication.h | 9 ++- .../Integration/ThreadingTests.cs | 58 +++++++++++++++++++ main/OpenCover.Test/OpenCover.Test.csproj | 1 + 4 files changed, 79 insertions(+), 21 deletions(-) create mode 100644 main/OpenCover.Test/Integration/ThreadingTests.cs diff --git a/main/OpenCover.Profiler/ProfilerCommunication.cpp b/main/OpenCover.Profiler/ProfilerCommunication.cpp index 92af54b6c..03ca3def2 100644 --- a/main/OpenCover.Profiler/ProfilerCommunication.cpp +++ b/main/OpenCover.Profiler/ProfilerCommunication.cpp @@ -194,35 +194,34 @@ bool ProfilerCommunication::Initialise(TCHAR *key, TCHAR *ns, TCHAR *processName } void ProfilerCommunication::ThreadCreated(ThreadID threadID, DWORD osThreadID){ - ATL::CComCritSecLock lock(_critThreads); _threadmap[threadID] = osThreadID; AllocateVisitMap(osThreadID); } MSG_SendVisitPoints_Request* ProfilerCommunication::AllocateVisitMap(DWORD osThreadID){ - auto p = new MSG_SendVisitPoints_Request(); - p->count = 0; - //::ZeroMemory(p, sizeof(MSG_SendVisitPoints_Request)); - _visitmap[osThreadID] = p; - return p; + ATL::CComCritSecLock lock(_critThreads); + auto it = _visitmap.find(osThreadID); + if (it == _visitmap.end() || it->second == nullptr) + { + auto p = new MSG_SendVisitPoints_Request(); + p->count = 0; + _visitmap[osThreadID] = p; + return p; + } + return it->second; } MSG_SendVisitPoints_Request* ProfilerCommunication::GetVisitMapForOSThread(ULONG osThreadID){ - MSG_SendVisitPoints_Request * p; auto it = _visitmap.find(osThreadID); - if (it == _visitmap.end()) { - ATL::CComCritSecLock lock(_critThreads); - it = _visitmap.find(osThreadID); - if (it == _visitmap.end()) { - return AllocateVisitMap(osThreadID); - } + if (it == _visitmap.end() || it->second == nullptr) { + return AllocateVisitMap(osThreadID); } return it->second; } void ProfilerCommunication::ThreadDestroyed(ThreadID threadID){ ATL::CComCritSecLock lock(_critThreads); - ULONG osThreadId = _threadmap[threadID]; + auto osThreadId = _threadmap[threadID]; auto points = _visitmap[osThreadId]; SendThreadVisitPoints(points); delete _visitmap[osThreadId]; @@ -230,24 +229,21 @@ void ProfilerCommunication::ThreadDestroyed(ThreadID threadID){ } void ProfilerCommunication::SendRemainingThreadBuffers(){ - ATL::CComCritSecLock lock(_critThreads); for (auto it = _visitmap.begin(); it != _visitmap.end(); ++it){ if (it->second != nullptr){ SendThreadVisitPoints(it->second); - //::ZeroMemory(pVisitPoints, sizeof(MSG_SendVisitPoints_Request)); } } } void ProfilerCommunication::AddVisitPointToThreadBuffer(ULONG uniqueId, MSG_IdType msgType) { - DWORD osThreadId = ::GetCurrentThreadId(); + auto osThreadId = ::GetCurrentThreadId(); auto pVisitPoints = GetVisitMapForOSThread(osThreadId); pVisitPoints->points[pVisitPoints->count].UniqueId = (uniqueId | msgType); if (++pVisitPoints->count == VP_BUFFER_SIZE) { SendThreadVisitPoints(pVisitPoints); - //::ZeroMemory(pVisitPoints, sizeof(MSG_SendVisitPoints_Request)); } } diff --git a/main/OpenCover.Profiler/ProfilerCommunication.h b/main/OpenCover.Profiler/ProfilerCommunication.h index c07341d29..04759ce92 100644 --- a/main/OpenCover.Profiler/ProfilerCommunication.h +++ b/main/OpenCover.Profiler/ProfilerCommunication.h @@ -11,7 +11,7 @@ #include -#include +#include /// Handles communication back to the profiler host /// Currently this is handled by using the WebServices API @@ -95,8 +95,11 @@ class ProfilerCommunication private: ATL::CComAutoCriticalSection _critThreads; - std::unordered_map _threadmap; - std::unordered_map _visitmap; + //std::unordered_map _threadmap; + //std::unordered_map _visitmap; + + Concurrency::concurrent_unordered_map _threadmap; + Concurrency::concurrent_unordered_map _visitmap; MSG_SendVisitPoints_Request* GetVisitMapForOSThread(ULONG osThread); diff --git a/main/OpenCover.Test/Integration/ThreadingTests.cs b/main/OpenCover.Test/Integration/ThreadingTests.cs new file mode 100644 index 000000000..af48c7148 --- /dev/null +++ b/main/OpenCover.Test/Integration/ThreadingTests.cs @@ -0,0 +1,58 @@ +using System; +using System.Diagnostics; +using System.Linq; +using System.Threading; +using NUnit.Framework; +using OpenCover.Framework; + +namespace OpenCover.Test.Integration +{ + /// + /// Replicates issue with threads as reported in issue #366 + /// + [TestFixture] + public class ThreadingTests + { + const int NB_THREADS = 50; + static readonly ManualResetEvent[] ResetEvents = new ManualResetEvent[NB_THREADS]; + + [Test] + public void RunManyThreads() + { + //Thread.Sleep(15000); + for (int i = 0; i < NB_THREADS; i++) + { + ResetEvents[i] = new ManualResetEvent(false); + new Thread(DoWork).Start(ResetEvents[i]); + } + var chrono = Stopwatch.StartNew(); + long n = 0; + while (n < 2000) + { + if (++n % 200 == 0) + Console.WriteLine(n.ToString()); + var current = WaitHandle.WaitAny(ResetEvents.ToArray()); + ResetEvents[current].Reset(); + new Thread(DoWork).Start(ResetEvents[current]); + } + Console.WriteLine("Took {0} seconds", chrono.Elapsed.TotalSeconds); + Assert.Pass(); + } + + public static void DoWork(object o) + { + var resetEvent = (ManualResetEvent)o; + resetEvent.Do(re => + { + var rnd = new Random(); + double res = 0; + for (var i = 0; i < 10000; i++) + res += rnd.NextDouble(); + re.Set(); + }); + + + } + + } +} diff --git a/main/OpenCover.Test/OpenCover.Test.csproj b/main/OpenCover.Test/OpenCover.Test.csproj index fb09b74b3..755e145ef 100644 --- a/main/OpenCover.Test/OpenCover.Test.csproj +++ b/main/OpenCover.Test/OpenCover.Test.csproj @@ -167,6 +167,7 @@ + From 1370d0469f9aaa8cf97de1ef26d0ad0a65c03aa6 Mon Sep 17 00:00:00 2001 From: sawilde Date: Mon, 21 Dec 2015 16:41:34 +0000 Subject: [PATCH 08/10] fix spelling mistake --- main/OpenCover.Console/Program.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main/OpenCover.Console/Program.cs b/main/OpenCover.Console/Program.cs index 1a23ecfac..328641ee3 100644 --- a/main/OpenCover.Console/Program.cs +++ b/main/OpenCover.Console/Program.cs @@ -74,7 +74,7 @@ static int Main(string[] args) { Logger.FatalFormat("An exception occured: {0}", ex.Message); Logger.FatalFormat("stack: {0}", ex.StackTrace); - Logger.FatalFormat("A report has been sent to the OenCover development team..."); + Logger.FatalFormat("A report has been sent to the OpenCover development team..."); } ReportCrash(ex); From f7aa1656cf96490c54594f324a7de04fae8148d4 Mon Sep 17 00:00:00 2001 From: sawilde Date: Mon, 21 Dec 2015 17:16:41 +0000 Subject: [PATCH 09/10] update notes --- ReleaseNotes.tmp | 1 + 1 file changed, 1 insertion(+) diff --git a/ReleaseNotes.tmp b/ReleaseNotes.tmp index f4a23025f..399c86ba9 100644 --- a/ReleaseNotes.tmp +++ b/ReleaseNotes.tmp @@ -1,5 +1,6 @@ Version [[version]] #376 protect buffer allocation in multithreaded environment (fix) +#335 allow short wait timeout to be configured (feature) Version 4.6.210 (rc - remove) #282 exclude by process (feature) From 54c551bc4efb45b505b5f29297dd1f28b63be510 Mon Sep 17 00:00:00 2001 From: sawilde Date: Mon, 21 Dec 2015 17:47:55 +0000 Subject: [PATCH 10/10] #366 add lock protection over around Module.Alias access by base persistence --- .../Persistance/BasePersistance.cs | 136 ++++++++++-------- 1 file changed, 75 insertions(+), 61 deletions(-) diff --git a/main/OpenCover.Framework/Persistance/BasePersistance.cs b/main/OpenCover.Framework/Persistance/BasePersistance.cs index 33d30a28c..ef41ef36a 100644 --- a/main/OpenCover.Framework/Persistance/BasePersistance.cs +++ b/main/OpenCover.Framework/Persistance/BasePersistance.cs @@ -1,8 +1,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Linq; -using System.Text.RegularExpressions; using OpenCover.Framework.Communication; using OpenCover.Framework.Model; using OpenCover.Framework.Utility; @@ -15,6 +13,9 @@ namespace OpenCover.Framework.Persistance /// public abstract class BasePersistance : IPersistance { + + private static readonly object Protection = new object(); + /// /// Provides subclasses access to the command line object /// @@ -53,15 +54,23 @@ public void PersistModule(Module module) module.Classes = module.Classes ?? new Class[0]; if (CommandLine.MergeByHash) { - var modules = CoverageSession.Modules ?? new Module[0]; - var existingModule = modules.FirstOrDefault(x => x.ModuleHash == module.ModuleHash); - if (existingModule!=null) + lock (Protection) { - if (!existingModule.Aliases.Any(x=>x.Equals(module.FullName, StringComparison.InvariantCultureIgnoreCase))) + var modules = CoverageSession.Modules ?? new Module[0]; + lock (Protection) { - existingModule.Aliases.Add(module.FullName); + var existingModule = modules.FirstOrDefault(x => x.ModuleHash == module.ModuleHash); + if (existingModule != null) + { + if ( + !existingModule.Aliases.Any( + x => x.Equals(module.FullName, StringComparison.InvariantCultureIgnoreCase))) + { + existingModule.Aliases.Add(module.FullName); + } + return; + } } - return; } } @@ -136,8 +145,10 @@ private void BuildMethodMapForModule(Module module) /// public bool IsTracking(string modulePath) { - return CoverageSession.Modules.Any(x => x.Aliases.Any(path => path.Equals(modulePath, StringComparison.InvariantCultureIgnoreCase)) && - !x.ShouldSerializeSkippedDueTo()); + lock (Protection) { + return CoverageSession.Modules.Any(x => x.Aliases.Any(path => path.Equals(modulePath, StringComparison.InvariantCultureIgnoreCase)) && + !x.ShouldSerializeSkippedDueTo()); + } } /// @@ -237,50 +248,47 @@ private void RemoveUnreferencedFiles() } // static readonly empty collections, saves creation time of new empty ones - private static readonly SequencePoint[] emptySeqPoints = new SequencePoint[0]; - private static readonly BranchPoint[] emptyBranchPoints = new BranchPoint[0]; - private static readonly List emptyBranchList = new List(0); + private static readonly SequencePoint[] EmptySeqPoints = new SequencePoint[0]; + private static readonly BranchPoint[] EmptyBranchPoints = new BranchPoint[0]; + private static readonly List EmptyBranchList = new List(0); // Dictionary with stored source files per module - private Dictionary sourceRepository = new Dictionary(); + private Dictionary _sourceRepository = new Dictionary(); - private const bool doRemove = true; - private const bool preserve = false; - - private uint fileID_cache = 0; - private CodeCoverageStringTextSource textSource_cache = null; - private CodeCoverageStringTextSource getCodeCoverageStringTextSource (uint fileId) { + private uint _fileIdCache; + private CodeCoverageStringTextSource _textSourceCache; + private CodeCoverageStringTextSource GetCodeCoverageStringTextSource (uint fileId) { CodeCoverageStringTextSource source = null; if (fileId != 0) { - if (fileID_cache == fileId) { - source = textSource_cache; + if (_fileIdCache == fileId) { + source = _textSourceCache; } else { - sourceRepository.TryGetValue (fileId, out source); + _sourceRepository.TryGetValue (fileId, out source); if (source != null) { - fileID_cache = fileId; - textSource_cache = source; + _fileIdCache = fileId; + _textSourceCache = source; } } } return source; } - private string getSequencePointText (SequencePoint sp) { + private string GetSequencePointText (SequencePoint sp) { if (sp != null) { - CodeCoverageStringTextSource source = getCodeCoverageStringTextSource (sp.FileId); + CodeCoverageStringTextSource source = GetCodeCoverageStringTextSource (sp.FileId); return source != null ? source.GetText(sp) : ""; } return ""; } - private static bool isSingleCharSequencePoint (SequencePoint sp) { + private static bool IsSingleCharSequencePoint (SequencePoint sp) { return ((sp != null) && (sp.StartLine == sp.EndLine) && (sp.EndColumn - sp.StartColumn) == 1); } - private bool isLeftBraceSequencePoint (SequencePoint sp) { - return isSingleCharSequencePoint(sp) && getSequencePointText(sp) == "{"; + private bool IsLeftBraceSequencePoint (SequencePoint sp) { + return IsSingleCharSequencePoint(sp) && GetSequencePointText(sp) == "{"; } - private bool isRightBraceSequencePoint (SequencePoint sp) { - return isSingleCharSequencePoint(sp) && getSequencePointText(sp) == "}"; + private bool IsRightBraceSequencePoint (SequencePoint sp) { + return IsSingleCharSequencePoint(sp) && GetSequencePointText(sp) == "}"; } private void PopulateInstrumentedPoints() @@ -305,13 +313,13 @@ from method in @class.Methods.Where(x => !x.ShouldSerializeSkippedDueTo()) #region Module FileID/FullPath/TextSource - sourceRepository = new Dictionary(); + _sourceRepository = new Dictionary(); var filesDictionary = new Dictionary(); foreach (var file in (module.Files ?? new File[0]).Where(file => !String.IsNullOrWhiteSpace(file.FullPath) && !filesDictionary.ContainsKey(file.FullPath))) { var source = CodeCoverageStringTextSource.GetSource(file.FullPath); - if (source != null) sourceRepository.Add (file.UniqueId, source); + if (source != null) _sourceRepository.Add (file.UniqueId, source); filesDictionary.Add(file.FullPath, file.UniqueId); } @@ -329,12 +337,12 @@ from method in @class.Methods.Where(x => !x.ShouldSerializeSkippedDueTo()) foreach (var method in (@class.Methods ?? new Method[0]).Where(x => !x.ShouldSerializeSkippedDueTo())) { - if (method.SequencePoints == null) method.SequencePoints = emptySeqPoints; - if (method.BranchPoints == null) method.BranchPoints = emptyBranchPoints; + if (method.SequencePoints == null) method.SequencePoints = EmptySeqPoints; + if (method.BranchPoints == null) method.BranchPoints = EmptyBranchPoints; // No sequences in method, but branches present? => remove branches if (method.SequencePoints.Length == 0 && method.BranchPoints.Length != 0) { - method.BranchPoints = emptyBranchPoints; + method.BranchPoints = EmptyBranchPoints; } if (method.SequencePoints.Length != 0) MapFileReferences(method.SequencePoints, filesDictionary); @@ -378,17 +386,17 @@ from method in @class.Methods.Where(x => !x.ShouldSerializeSkippedDueTo()) long startOffset = long.MinValue; long finalOffset = long.MaxValue; - CodeCoverageStringTextSource source = getCodeCoverageStringTextSource(method.FileRef.UniqueId); + CodeCoverageStringTextSource source = GetCodeCoverageStringTextSource(method.FileRef.UniqueId); if (source != null && source.FileType == FileType.CSharp) { var sourceLineOrderedSps = method.SequencePoints.OrderBy(sp=>sp.StartLine).ThenBy(sp=>sp.StartColumn).Where(sp=>sp.FileId == method.FileRef.UniqueId).ToArray(); if (sourceLineOrderedSps.Length >= 3) { // method.sp; leftBrace.sp, rightBrace.sp || leftBrace.sp, any.sp, rightBrace.sp - if (isLeftBraceSequencePoint(sourceLineOrderedSps[1])) { + if (IsLeftBraceSequencePoint(sourceLineOrderedSps[1])) { startOffset = sourceLineOrderedSps[1].Offset; } - else if (isLeftBraceSequencePoint(sourceLineOrderedSps[0])) { + else if (IsLeftBraceSequencePoint(sourceLineOrderedSps[0])) { startOffset = sourceLineOrderedSps[0].Offset; } - if (isRightBraceSequencePoint(sourceLineOrderedSps.Last())) { + if (IsRightBraceSequencePoint(sourceLineOrderedSps.Last())) { finalOffset = sourceLineOrderedSps.Last().Offset; } } @@ -400,7 +408,7 @@ from method in @class.Methods.Where(x => !x.ShouldSerializeSkippedDueTo()) foreach (var sp in method.SequencePoints) { if (sp != null && sp.BranchPoints != null && sp.BranchPoints.Count != 0 && sp.FileId == method.FileRef.UniqueId) { if (sp.Offset <= startOffset || finalOffset <= sp.Offset) { - sp.BranchPoints = emptyBranchList; + sp.BranchPoints = EmptyBranchList; } } } @@ -439,7 +447,7 @@ from method in @class.Methods.Where(x => !x.ShouldSerializeSkippedDueTo()) // Add to validBranchPoints validBranchPoints.AddRange(sp.BranchPoints); - sp.BranchPoints = emptyBranchList; // clear + sp.BranchPoints = EmptyBranchList; // clear } } @@ -447,7 +455,7 @@ from method in @class.Methods.Where(x => !x.ShouldSerializeSkippedDueTo()) // Order is Required by FilePersistanceTest because it does not sets .Offset. // (Order by UniqueSequencePoint is equal to order by .Offset when .Offset is set) method.BranchPoints = validBranchPoints.OrderBy(bp => bp.UniqueSequencePoint).ToArray(); - validBranchPoints = emptyBranchList; // clear + validBranchPoints = EmptyBranchList; // clear #endregion @@ -515,7 +523,7 @@ from method in @class.Methods.Where(x => !x.ShouldSerializeSkippedDueTo()) CoverageSession.Summary.MaxCyclomaticComplexity = Math.Max(CoverageSession.Summary.MaxCyclomaticComplexity, module.Summary.MaxCyclomaticComplexity); filesDictionary.Clear(); - sourceRepository.Clear(); + _sourceRepository.Clear(); } CalculateCoverage(CoverageSession.Summary); @@ -612,13 +620,17 @@ private Method GetMethod(string modulePath, int functionToken, out Class @class) { @class = null; //c = null; - var module = CoverageSession.Modules.FirstOrDefault(x => x.Aliases.Any(path => path.Equals(modulePath, StringComparison.InvariantCultureIgnoreCase))); - if (module == null) - return null; - if (!_moduleMethodMap[module].ContainsKey(functionToken)) return null; - var pair = _moduleMethodMap[module][functionToken]; - @class = pair.Key; - return pair.Value; + lock (Protection) + { + var module = CoverageSession.Modules + .FirstOrDefault(x => x.Aliases.Any(path => path.Equals(modulePath, StringComparison.InvariantCultureIgnoreCase))); + if (module == null) + return null; + if (!_moduleMethodMap[module].ContainsKey(functionToken)) return null; + var pair = _moduleMethodMap[module][functionToken]; + @class = pair.Key; + return pair.Value; + } } /// @@ -676,21 +688,23 @@ public void SaveVisitData(byte[] data) /// public bool GetTrackingMethod(string modulePath, string assemblyName, int functionToken, out uint uniqueId) { - uniqueId = 0; - foreach (var module in CoverageSession.Modules - .Where(x => x.TrackedMethods != null) - .Where(x => x.Aliases.Contains(modulePath))) + lock (Protection) { - foreach (var trackedMethod in module.TrackedMethods) + uniqueId = 0; + foreach (var module in CoverageSession.Modules + .Where(x => x.TrackedMethods != null) + .Where(x => x.Aliases.Contains(modulePath))) { - if (trackedMethod.MetadataToken == functionToken) + foreach (var trackedMethod in module.TrackedMethods) { - uniqueId = trackedMethod.UniqueId; - return true; + if (trackedMethod.MetadataToken == functionToken) + { + uniqueId = trackedMethod.UniqueId; + return true; + } } } } - return false; } }