Prevent Flush from deadlocking

DesktopAnalytics.sln

@@ -23,7 +23,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "DesktopAnalyticsTests", "src\DesktopAnalyticsTests\DesktopAnalyticsTests.csproj", "{652BCDED-1C97-4BAC-8292-3A3EF605DC59}"
 EndProject
-Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SampleAppWithForm", "SampleAppWithForm\SampleAppWithForm.csproj", "{3B405602-752F-4A3D-B31D-DBFB2DB6BEBD}"
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SampleAppWithForm", "src\SampleAppWithForm\SampleAppWithForm.csproj", "{471C3C0A-EA78-4D2A-AC47-2129A3A91C6D}"
 EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
@@ -55,14 +55,14 @@ Global
 		{652BCDED-1C97-4BAC-8292-3A3EF605DC59}.Release|Any CPU.Build.0 = Release|Any CPU
 		{652BCDED-1C97-4BAC-8292-3A3EF605DC59}.Release|x86.ActiveCfg = Release|Any CPU
 		{652BCDED-1C97-4BAC-8292-3A3EF605DC59}.Release|x86.Build.0 = Release|Any CPU
-		{3B405602-752F-4A3D-B31D-DBFB2DB6BEBD}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
-		{3B405602-752F-4A3D-B31D-DBFB2DB6BEBD}.Debug|Any CPU.Build.0 = Debug|Any CPU
-		{3B405602-752F-4A3D-B31D-DBFB2DB6BEBD}.Debug|x86.ActiveCfg = Debug|Any CPU
-		{3B405602-752F-4A3D-B31D-DBFB2DB6BEBD}.Debug|x86.Build.0 = Debug|Any CPU
-		{3B405602-752F-4A3D-B31D-DBFB2DB6BEBD}.Release|Any CPU.ActiveCfg = Release|Any CPU
-		{3B405602-752F-4A3D-B31D-DBFB2DB6BEBD}.Release|Any CPU.Build.0 = Release|Any CPU
-		{3B405602-752F-4A3D-B31D-DBFB2DB6BEBD}.Release|x86.ActiveCfg = Release|Any CPU
-		{3B405602-752F-4A3D-B31D-DBFB2DB6BEBD}.Release|x86.Build.0 = Release|Any CPU
+		{471C3C0A-EA78-4D2A-AC47-2129A3A91C6D}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{471C3C0A-EA78-4D2A-AC47-2129A3A91C6D}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{471C3C0A-EA78-4D2A-AC47-2129A3A91C6D}.Debug|x86.ActiveCfg = Debug|Any CPU
+		{471C3C0A-EA78-4D2A-AC47-2129A3A91C6D}.Debug|x86.Build.0 = Debug|Any CPU
+		{471C3C0A-EA78-4D2A-AC47-2129A3A91C6D}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{471C3C0A-EA78-4D2A-AC47-2129A3A91C6D}.Release|Any CPU.Build.0 = Release|Any CPU
+		{471C3C0A-EA78-4D2A-AC47-2129A3A91C6D}.Release|x86.ActiveCfg = Release|Any CPU
+		{471C3C0A-EA78-4D2A-AC47-2129A3A91C6D}.Release|x86.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE

DesktopAnalytics.sln.DotSettings

@@ -0,0 +1,4 @@
+<wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">
+	<s:Boolean x:Key="/Default/UserDictionary/Words/=Analytics/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/UserDictionary/Words/=segmentio/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/UserDictionary/Words/=unmanifested/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>

src/DesktopAnalytics/Analytics.cs

@@ -10,10 +10,10 @@
 using System.Net;
 using System.Reflection;
 using System.Runtime.InteropServices;
-using System.Runtime.Remoting.Contexts;
+using System.Threading;
 using System.Xml.Linq;
 using System.Xml.XPath;
-using Newtonsoft.Json;
+using JetBrains.Annotations;
 using Newtonsoft.Json.Linq;
 using Segment.Model;
 
@@ -41,8 +41,9 @@ public class Analytics : IDisposable
 		private static Traits _traits;
 		private static UserInfo _userInfo;
 		private static Analytics _singleton;
-		private Dictionary<string, string> _propertiesThatGoWithEveryEvent;
+		private readonly Dictionary<string, string> _propertiesThatGoWithEveryEvent;
 		private static int _exceptionCount = 0;
+
 		const int MAX_EXCEPTION_REPORTS_PER_RUN = 10;
 
 		public Analytics(string apiSecret, UserInfo userInfo, bool allowTracking = true, bool retainPii = false)
@@ -104,6 +105,11 @@ public Analytics(string apiSecret, UserInfo userInfo, Dictionary<string, string>
 			}
 
 			Segment.Analytics.Initialize(apiSecret);
+			// All these were attempts to prevent deadlock when calling Flush in response to
+			// the main window closing
+			//Segment.Analytics.Client.Config.SetTimeout(TimeSpan.FromMilliseconds(5000));
+			//Segment.Analytics.Client.Config.SetMaxRetryTime(TimeSpan.FromMilliseconds(7500));
+			//Segment.Analytics.Client.Config.SetThreads(2);
 			Segment.Analytics.Client.Failed += Client_Failed;
 			Segment.Analytics.Client.Succeeded += Client_Succeeded;
 
@@ -180,11 +186,10 @@ private void AttemptToGetUserIdSettingsFromDifferentChannel()
 			// which should work for everyone (though until there is a plugin that supports channels, it will presumably
 			// never actually find a pre-existing config file from a different channel).
 
-			string settingsLocation;
-			string softwareName;
-
 			for (int attempt = 0; attempt < 2; attempt++)
 			{
+				string settingsLocation;
+				string softwareName;
 				if (attempt == 0)
 				{
 					if (!TryGetSettingsLocationInfoFromEntryAssembly(out settingsLocation, out softwareName))
@@ -357,6 +362,7 @@ private static void UpdateSegmentIOInformationOnThisUser()
 		/// <summary>
 		/// Use this after showing a registration dialog, so that this stuff is sent right away, rather than the next time you start up Analytics
 		/// </summary>
+		[PublicAPI]
 		public static void IdentifyUpdate(UserInfo userInfo)
 		{
 			_userInfo = userInfo;
@@ -383,9 +389,8 @@ private void ReportIpAddressOfThisMachineAsync()
 			{
 				try
 				{
-					Uri uri;
 					bool json = string.IsNullOrEmpty(UrlThatReturnsExternalIpAddress);
-					Uri.TryCreate(json ? UrlThatReturnsGeolocationJson : UrlThatReturnsExternalIpAddress, UriKind.Absolute, out uri);
+					Uri.TryCreate(json ? UrlThatReturnsGeolocationJson : UrlThatReturnsExternalIpAddress, UriKind.Absolute, out var uri);
 					client.DownloadDataCompleted += (object sender, DownloadDataCompletedEventArgs e) =>
 					{
 						var launchProperties = new Properties { { "installedUiLangId", CultureInfo.InstalledUICulture.ThreeLetterISOLanguageName } };
@@ -395,15 +400,15 @@ private void ReportIpAddressOfThisMachineAsync()
 							var result = System.Text.Encoding.UTF8.GetString(e.Result).Trim();
 							if (json)
 							{
-								Debug.WriteLine(String.Format("DesktopAnalytics: geolocation JSON data = {0}", result));
+								Debug.WriteLine($"DesktopAnalytics: geolocation JSON data = {result}");
 								var j = JObject.Parse(result);
 								AddGeolocationProperty(j, "city");
 								AddGeolocationProperty(j, "country", "countryCode");
 								AddGeolocationProperty(j, "regionName", "region");
 							}
 							else
 							{
-								Debug.WriteLine(String.Format("DesktopAnalytics: external ip = {0}", result));
+								Debug.WriteLine($"DesktopAnalytics: external ip = {result}");
 								_options.Context.Add("ip", result);
 								_propertiesThatGoWithEveryEvent.Add("ip", result);
 							}
@@ -452,10 +457,16 @@ private IEnumerable<KeyValuePair<string, string>> GetLocationPropertiesOfThisMac
 		}
 
 		/// <summary>
-		/// Record an event
+		/// Records an event
 		/// </summary>
-		///	Analytics.RecordEvent("Save PDF");
-		/// <param name="eventName"></param>
+		/// <example>
+		///	Analytics.Track("Save PDF");
+		///	</example>
+		/// <param name="eventName">A good event name should be meaningful to a developer or
+		/// analyst, relatively short, and unique within an app. (Of course, you can issue the
+		/// same event from multiple places in your application code if they should be treated as
+		/// equivalent events.)</param>
+		[PublicAPI]
 		public static void Track(string eventName)
 		{
 			if (!AllowTracking)
@@ -468,14 +479,18 @@ public static void Track(string eventName)
 		/// Record an event with extra properties
 		/// </summary>
 		/// <example>
-		///	Analytics.RecordEvent("Save PDF", new Dictionary<string, string>()
+		///	Analytics.Track("Save PDF", new Dictionary&lt;string, string&gt;()
 		/// {
 		///		{"Portion",  Enum.GetName(typeof(BookletPortions), BookletPortion)},
 		///		{"Layout", PageLayout.ToString()}
 		///	});
 		///	</example>
-		/// <param name="eventName"></param>
-		/// <param name="properties"></param>
+		/// <param name="eventName">A good event name should be meaningful to a developer or
+		/// analyst, relatively short, and unique within an app. (Of course, you can issue the
+		/// same event from multiple places in your application code if they should be treated as
+		/// equivalent events.)</param>
+		/// <param name="properties">A dictionary of key-value pairs that are relevant in
+		/// understanding more about the event being tracked.</param>
 		public static void Track(string eventName, Dictionary<string, string> properties)
 		{
 			if (!AllowTracking)
@@ -488,6 +503,7 @@ public static void Track(string eventName, Dictionary<string, string> properties
 		/// Sends the exception's message and stacktrace
 		/// </summary>
 		/// <param name="e"></param>
+		[PublicAPI]
 		public static void ReportException(Exception e)
 		{
 			ReportException(e, null);
@@ -555,8 +571,28 @@ public void Dispose()
 				// The documentation is ambiguous about whether Flush() needs to be called before Dispose(),
 				// but source code at https://github.com/segmentio/Analytics.NET/blob/master/Analytics/Client.cs
 				// clearly says "Note, this does not call Flush() first".
-				// So to be sure of getting all our events we should call it.
-				Segment.Analytics.Client.Flush();
+				// So to be sure of getting all our events we should call it. Unfortunately, if Flush is called
+				// in response to the main application window closing, it can cause deadlock, and the app hangs.
+				// https://github.com/segmentio/Analytics.NET/issues/200
+				// So instead of calling Flush, if there are events in the queue, we just wait a little while.
+				// The default timeout on the client is 5 seconds, so probably we should never need to wait
+				// longer than that.
+				var stats = Segment.Analytics.Client.Statistics;
+				int totalWait = 0;
+				while (stats.Submitted > stats.Failed + stats.Succeeded)
+				{
+					if (totalWait > 7500)
+						break;
+					// This might seem like a long time to wait, but
+					// a) it will only have to do this is there are unsent events in the queue
+					// b) trying to wait less time doesn't seem to make it go any faster. (I did
+					//    try to cut the timeout time way down in hopes that that would cause
+					//    Segment to process the queue immediately, but it seemed to have no
+					//    effect.)
+					totalWait += 500;
+					Thread.Sleep(500);
+				}
+				//Segment.Analytics.Client.Flush();
 				Segment.Analytics.Client.Dispose();
 			}
 		}
@@ -572,7 +608,7 @@ class Version
 			private readonly PlatformID _platform;
 			private readonly int _major;
 			private readonly int _minor;
-			public string Label { get; private set; }
+			public string Label { get; }
 
 			public Version(PlatformID platform, int major, int minor, string label)
 			{
@@ -593,19 +629,21 @@ private static string GetOperatingSystemLabel()
 		{
 			if (Environment.OSVersion.Platform == PlatformID.Unix)
 			{
-				return UnixName == "Linux" ? String.Format("{0} / {1}", LinuxVersion, LinuxDesktop) : UnixName;
+				return UnixName == "Linux" ? $"{LinuxVersion} / {LinuxDesktop}" : UnixName;
 			}
-			var list = new List<Version>();
-			list.Add(new Version(System.PlatformID.Win32NT, 5, 0, "Windows 2000"));
-			list.Add(new Version(System.PlatformID.Win32NT, 5, 1, "Windows XP"));
-			list.Add(new Version(System.PlatformID.Win32NT, 6, 0, "Vista"));
-			list.Add(new Version(System.PlatformID.Win32NT, 6, 1, "Windows 7"));
-			// After Windows 8 the Environment.OSVersion started misreporting information unless
-			// your app has a manifest which says it supports the OS it is running on.  This is not
-			// helpful if someone starts using an app built before the OS is released. Anything that
-			// reports its self as Windows 8 is suspect, and must get the version info another way.
-			list.Add(new Version(PlatformID.Win32NT, 6, 3, "Windows 8.1"));
-			list.Add(new Version(PlatformID.Win32NT, 10, 0, "Windows 10"));
+			var list = new List<Version>
+			{
+				new Version(PlatformID.Win32NT, 5, 0, "Windows 2000"),
+				new Version(PlatformID.Win32NT, 5, 1, "Windows XP"),
+				new Version(PlatformID.Win32NT, 6, 0, "Vista"),
+				new Version(PlatformID.Win32NT, 6, 1, "Windows 7"),
+				// After Windows 8 the Environment.OSVersion started misreporting information unless
+				// your app has a manifest which says it supports the OS it is running on. This is not
+				// helpful if someone starts using an app built before the OS is released. Anything that
+				// reports itself as Windows 8 is suspect, and must get the version info another way.
+				new Version(PlatformID.Win32NT, 6, 3, "Windows 8.1"),
+				new Version(PlatformID.Win32NT, 10, 0, "Windows 10")
+			};
 
 			foreach (var version in list)
 			{
@@ -725,11 +763,11 @@ private static string LinuxVersion
 					{
 						var versionData = File.ReadAllText("/etc/wasta-release");
 						var versionLines = versionData.Split(new char[] { '\n' }, StringSplitOptions.RemoveEmptyEntries);
-						for (int i = 0; i < versionLines.Length; ++i)
+						foreach (var line in versionLines)
 						{
-							if (versionLines[i].StartsWith("DESCRIPTION=\""))
+							if (line.StartsWith("DESCRIPTION=\""))
 							{
-								_linuxVersion = versionLines[i].Substring(13).Trim(new char[] { '"' });
+								_linuxVersion = line.Substring(13).Trim('"');
 								break;
 							}
 						}
@@ -811,7 +849,7 @@ private static string LinuxDesktop
 					if (!string.IsNullOrEmpty(mirSession))
 						additionalInfo = " [display server: Mir]";
 					var gdmSession = Environment.GetEnvironmentVariable("GDMSESSION") ?? "not set";
-					_linuxDesktop = String.Format("{0} ({1}{2})", currentDesktop, gdmSession, additionalInfo);
+					_linuxDesktop = $"{currentDesktop} ({gdmSession}{additionalInfo})";
 				}
 				return _linuxDesktop;
 			}

src/DesktopAnalytics/DesktopAnalytics.csproj

@@ -15,10 +15,13 @@
     <SymbolPackageFormat>snupkg</SymbolPackageFormat>
 </PropertyGroup>
 <ItemGroup>
+    <PackageReference Include="JetBrains.Annotations" Version="2022.1.0">
+      <PrivateAssets>All</PrivateAssets>
+    </PackageReference>
     <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.1.1" PrivateAssets="All" />
 </ItemGroup>
 <ItemGroup>
-  <PackageReference Include="Analytics" Version="3.6.0" />
+  <PackageReference Include="Analytics" Version="3.8.1" />
   <PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
   <PackageReference Include="GitVersion.MsBuild" Version="5.10.3">
     <PrivateAssets>all</PrivateAssets>

src/SampleApp/SampleApp.csproj

@@ -9,7 +9,7 @@
   <IsPackable>false</IsPackable>
 </PropertyGroup>
 <ItemGroup>
-  <PackageReference Include="Analytics" Version="3.6.0" />
+  <PackageReference Include="Analytics" Version="3.8.1" />
   <PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
 </ItemGroup>
 <ItemGroup>

SampleAppWithForm/Form1.cs → src/SampleAppWithForm/Form1.cs

@@ -1,5 +1,7 @@
+using System;
+using System.Collections.Generic;
 using System.Diagnostics;
-using DesktopAnalytics;
+using System.Windows.Forms;
 
 namespace SampleAppWithForm
 {
@@ -28,7 +30,14 @@ private void Form1_FormClosed(object sender, FormClosedEventArgs e)
 			Debug.WriteLine($"Succeeded: {Segment.Analytics.Client.Statistics.Succeeded}; " +
 				$"Submitted: {Segment.Analytics.Client.Statistics.Submitted}; " +
 				$"Failed:  {Segment.Analytics.Client.Statistics.Failed}");
-			Program.s_analyticsSingleton.Dispose();
+			// This allows us to illustrate the deadlock problem:
+			// https://github.com/segmentio/Analytics.NET/issues/200
+			if (_chkFlush.Checked) 
+				Segment.Analytics.Client.Flush();
+			var stopwatch = Stopwatch.StartNew();
+			Program.s_analyticsSingleton?.Dispose();
+			stopwatch.Stop();
+			Debug.WriteLine($"Total wait = {stopwatch.Elapsed}");
 		}
 	}
 }

SampleAppWithForm/Program.cs → src/SampleAppWithForm/Program.cs

@@ -1,3 +1,6 @@
+using System;
+using System.Collections.Generic;
+using System.Windows.Forms;
 using DesktopAnalytics;
 
 namespace SampleAppWithForm
@@ -11,8 +14,6 @@ internal static class Program
 		[STAThread]
 		static void Main(string[] args)
 		{
-			ApplicationConfiguration.Initialize();
-
 			if (args.Length == 0)
 				Console.WriteLine("Usage: SampleApp <segmentioApiSecret>");
 
@@ -29,7 +30,9 @@ static void Main(string[] args)
 
 			var propertiesThatGoWithEveryEvent = new Dictionary<string, string> {{"channel", "beta"}};
 			s_analyticsSingleton = new Analytics(args[0], userInfo, propertiesThatGoWithEveryEvent);
-			Application.Run(new Form1());
+
+			var mainWindow = new Form1();
+			Application.Run(mainWindow);
 		}
 	}
 }