Merge pull request #76 from synhershko/54-timeout-error-handling

#54 Updater flow refactor and improvements

FeedBuilder/FeedBuilder.csproj

@@ -12,7 +12,7 @@
     <AssemblyName>FeedBuilder</AssemblyName>
     <FileAlignment>512</FileAlignment>
     <MyType>WindowsForms</MyType>
-    <TargetFrameworkVersion>v2.0</TargetFrameworkVersion>
+    <TargetFrameworkVersion>v3.5</TargetFrameworkVersion>
     <TargetFrameworkProfile>
     </TargetFrameworkProfile>
     <NoWarn>1591</NoWarn>
@@ -136,6 +136,7 @@
     </EmbeddedResource>
   </ItemGroup>
   <ItemGroup>
+    <None Include="app.config" />
     <None Include="Properties\Settings.settings">
       <Generator>SettingsSingleFileGenerator</Generator>
       <LastGenOutput>Settings.Designer.cs</LastGenOutput>
@@ -153,4 +154,4 @@
     </ProjectReference>
   </ItemGroup>
   <Import Project="$(MSBuildBinPath)\Microsoft.CSharp.Targets" />
-</Project>
+</Project>

FeedBuilder/app.config

@@ -0,0 +1,3 @@
+<?xml version="1.0" encoding="utf-8"?>
+<configuration>
+<startup><supportedRuntime version="v2.0.50727"/></startup></configuration>

src/NAppUpdate.Framework/NAppUpdate.Framework.csproj

@@ -10,7 +10,7 @@
     <AppDesignerFolder>Properties</AppDesignerFolder>
     <RootNamespace>NAppUpdate.Framework</RootNamespace>
     <AssemblyName>NAppUpdate.Framework</AssemblyName>
-    <TargetFrameworkVersion>v2.0</TargetFrameworkVersion>
+    <TargetFrameworkVersion>v3.5</TargetFrameworkVersion>
     <FileAlignment>512</FileAlignment>
     <SignAssembly>true</SignAssembly>
     <AssemblyOriginatorKeyFile>NAppUpdate.snk</AssemblyOriginatorKeyFile>
@@ -33,6 +33,7 @@
     <IsWebBootstrapper>false</IsWebBootstrapper>
     <UseApplicationTrust>false</UseApplicationTrust>
     <BootstrapperEnabled>true</BootstrapperEnabled>
+    <TargetFrameworkProfile />
   </PropertyGroup>
   <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
     <DebugSymbols>true</DebugSymbols>
@@ -101,6 +102,7 @@
     <Compile Include="Utils\NauIpc.cs" />
     <Compile Include="Utils\FileSystem.cs" />
     <Compile Include="Utils\PermissionsCheck.cs" />
+    <Compile Include="Utils\ProcessStartFailedException.cs" />
     <Compile Include="Utils\Reflection.cs" />
   </ItemGroup>
   <ItemGroup>

src/NAppUpdate.Framework/UpdateManager.cs

@@ -506,10 +506,20 @@ public void ApplyUpdates(bool relaunchApplication, bool updaterDoLogging, bool u
 
 		public void ReinstateIfRestarted()
 		{
+			if (!IsAfterRestart())
+			{
+				return;
+			}
+
 			lock (UpdatesToApply)
 			{
-				var dto = NauIpc.ReadDto(Config.UpdateProcessName) as NauIpc.NauDto;
-				if (dto == null) return;
+				NauIpc.NauDto dto = NauIpc.ReadDto(Config.UpdateProcessName);
+
+				if (dto == null)
+				{
+					return;
+				}
+
 				Config = dto.Configs;
 				UpdatesToApply = dto.Tasks;
 				Logger = new Logger(dto.LogItems);
@@ -581,5 +591,18 @@ public void CleanUp()
 				ShouldStop = false;
 			}
 		}
+
+		private bool IsAfterRestart()
+		{
+			foreach (string arg in Environment.GetCommandLineArgs())
+			{
+				if (arg == "-nappupdate-afterrestart")
+				{
+					return true;
+				}
+			}
+
+			return false;
+		}
 	}
 }

src/NAppUpdate.Framework/Utils/NauIpc.cs

@@ -9,6 +9,7 @@
 using System.Runtime.Serialization.Formatters.Binary;
 using NAppUpdate.Framework.Common;
 using NAppUpdate.Framework.Tasks;
+using System.IO.Pipes;
 
 namespace NAppUpdate.Framework.Utils
 {
@@ -29,158 +30,76 @@ internal class NauDto
 			public bool RelaunchApplication { get; set; }
 		}
 
-		[DllImport("kernel32.dll", SetLastError = true)]
-		private static extern SafeFileHandle CreateNamedPipe(
-			String pipeName,
-			uint dwOpenMode,
-			uint dwPipeMode,
-			uint nMaxInstances,
-			uint nOutBufferSize,
-			uint nInBufferSize,
-			uint nDefaultTimeOut,
-			IntPtr lpSecurityAttributes);
-
-		[DllImport("kernel32.dll", SetLastError = true)]
-		private static extern int ConnectNamedPipe(
-			SafeFileHandle hNamedPipe,
-			IntPtr lpOverlapped);
-
-		[DllImport("kernel32.dll", SetLastError = true)]
-		private static extern SafeFileHandle CreateFile(
-			String pipeName,
-			uint dwDesiredAccess,
-			uint dwShareMode,
-			IntPtr lpSecurityAttributes,
-			uint dwCreationDisposition,
-			uint dwFlagsAndAttributes,
-			IntPtr hTemplate);
-
-		//private const uint DUPLEX = (0x00000003);
-		private const uint WRITE_ONLY = (0x00000002);
-		private const uint FILE_FLAG_OVERLAPPED = (0x40000000);
-
-		const uint GENERIC_READ = (0x80000000);
-		//static readonly uint GENERIC_WRITE = (0x40000000);
-		const uint OPEN_EXISTING = 3;
-
-		//Which really isn't an error...
-		const uint ERROR_PIPE_CONNECTED = 535;
-
-		internal static string GetPipeName(string syncProcessName)
-		{
-			return string.Format("\\\\.\\pipe\\{0}", syncProcessName);
-		}
-
-		private class State
-		{
-			public readonly EventWaitHandle eventWaitHandle;
-			public int result { get; set; }
-			public SafeFileHandle clientPipeHandle { get; set; }
-			public Exception exception;
-
-			public State()
-			{
-				eventWaitHandle = new ManualResetEvent(false);
-			}
-		}
-
-		internal static uint BUFFER_SIZE = 4096;
+		private const int PIPE_TIMEOUT = 15000;
 
+		/// <summary>
+		/// Launches the specifies process and sends the dto object to it using a named pipe
+		/// </summary>
+		/// <param name="dto">Dto object to send</param>
+		/// <param name="processStartInfo">Process info for the process to start</param>
+		/// <param name="syncProcessName">Name of the pipe to write to</param>
+		/// <returns>The started process</returns>
 		public static Process LaunchProcessAndSendDto(NauDto dto, ProcessStartInfo processStartInfo, string syncProcessName)
 		{
 			Process p;
-			State state = new State();
-
-			using (state.clientPipeHandle = CreateNamedPipe(
-					GetPipeName(syncProcessName),
-					WRITE_ONLY | FILE_FLAG_OVERLAPPED,
-					0,
-					1, // 1 max instance (only the updater utility is expected to connect)
-					BUFFER_SIZE,
-					BUFFER_SIZE,
-					0,
-					IntPtr.Zero))
+
+			using (NamedPipeServerStream pipe = new NamedPipeServerStream(syncProcessName, PipeDirection.Out, 1, PipeTransmissionMode.Message, PipeOptions.Asynchronous))
 			{
-				//failed to create named pipe
-				if (state.clientPipeHandle.IsInvalid)
+				p = Process.Start(processStartInfo);
+
+				if (p == null)
 				{
-					throw new Exception("Launch process client: Failed to create named pipe, handle is invalid.");
+					throw new ProcessStartFailedException("The process failed to start");
 				}
 
-				// This will throw Win32Exception if the user denies UAC
-				p = Process.Start(processStartInfo);
-
-				ThreadPool.QueueUserWorkItem(ConnectPipe, state);
-				//A rather arbitary five seconds, perhaps better to be user configurable at some point?
-				state.eventWaitHandle.WaitOne(10000);
+				var asyncResult = pipe.BeginWaitForConnection(null, null);
 
-				//failed to connect client pipe
-				if (state.result == 0)
+				if (asyncResult.AsyncWaitHandle.WaitOne(PIPE_TIMEOUT))
 				{
-					throw new Exception("Launch process client: Failed to connect to named pipe", state.exception);
+					pipe.EndWaitForConnection(asyncResult);
+
+					BinaryFormatter formatter = new BinaryFormatter();
+					formatter.Serialize(pipe, dto);
 				}
+				else if (p.HasExited)
+				{
+					Type exceptionType = Marshal.GetExceptionForHR(p.ExitCode).GetType();
 
-				//client connection successfull
-				using (var fStream = new FileStream(state.clientPipeHandle, FileAccess.Write, (int)BUFFER_SIZE, true))
+					throw new TimeoutException(string.Format("The NamedPipeServerStream timed out waiting for a named pipe connection, " +
+						"but the process has exited with exit code: {0} ({1})", p.ExitCode, exceptionType.FullName));
+				}
+				else
 				{
-					new BinaryFormatter().Serialize(fStream, dto);
-					fStream.Flush();
-					fStream.Close();
+					throw new TimeoutException("The NamedPipeServerStream timed out waiting for a named pipe connection.");
 				}
 			}
 
 			return p;
 		}
 
-		internal static void ConnectPipe(object stateObject)
+		/// <summary>
+		/// Reads the dto object from the named pipe
+		/// </summary>
+		/// <param name="syncProcessName">Name of the pipe to read from</param>
+		/// <returns>The dto object read from the pipe</returns>
+		public static NauDto ReadDto(string syncProcessName)
 		{
-			State state = (State)stateObject;
+			NauDto dto;
 
-			try
-			{
-				state.result = ConnectNamedPipe(state.clientPipeHandle, IntPtr.Zero);
-			}
-			catch (Exception e)
+			using (NamedPipeClientStream pipe = new NamedPipeClientStream(".", syncProcessName, PipeDirection.In, PipeOptions.Asynchronous))
 			{
-				state.exception = e;
-			}
+				pipe.Connect(PIPE_TIMEOUT);
 
-			int error = Marshal.GetLastWin32Error();
-			//Check for the oddball: ERROR - PIPE CONNECTED
-			//Ref: http://msdn.microsoft.com/en-us/library/windows/desktop/aa365146%28v=vs.85%29.aspx
-			if (error == ERROR_PIPE_CONNECTED)
-			{
-				state.result = 1;
-			}
-			else if (error != 0)
-			{
-				state.exception = new Win32Exception(error);
+				BinaryFormatter formatter = new BinaryFormatter();
+				dto = formatter.Deserialize(pipe) as NauDto;
 			}
 
-			state.eventWaitHandle.Set(); // signal we're done
-		}
-
-
-		internal static object ReadDto(string syncProcessName)
-		{
-			using (SafeFileHandle pipeHandle = CreateFile(
-				GetPipeName(syncProcessName),
-				GENERIC_READ,
-				0,
-				IntPtr.Zero,
-				OPEN_EXISTING,
-				FILE_FLAG_OVERLAPPED,
-				IntPtr.Zero))
+			if (dto == null || dto.Configs == null)
 			{
-
-				if (pipeHandle.IsInvalid)
-					return null;
-
-				using (var fStream = new FileStream(pipeHandle, FileAccess.Read, (int)BUFFER_SIZE, true))
-				{
-					return new BinaryFormatter().Deserialize(fStream);
-				}
+				throw new Exception("Failed to read the dto from the pipe stream");
 			}
+
+			return dto;
 		}
 
 		internal static void ExtractUpdaterFromResource(string updaterPath, string hostExeName)
@@ -212,4 +131,4 @@ internal static void ExtractUpdaterFromResource(string updaterPath, string hostE
 			}
 		}
 	}
-}
+}

src/NAppUpdate.Framework/Utils/ProcessStartFailedException.cs

@@ -0,0 +1,16 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+
+namespace NAppUpdate.Framework.Utils
+{
+	/// <summary>
+	/// Thrown if the Process.Start() call returns null, e.g. the updater process fails to start at all.
+	/// </summary>
+	internal class ProcessStartFailedException : Exception
+	{
+		public ProcessStartFailedException(string message) : base(message) { }
+	}
+
+}