From ee2f911a2b85792c2bfc867d42b7d58b847de6ea Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Fri, 27 Mar 2020 00:10:16 +0100 Subject: [PATCH] Remove unnecessary CommonProcess abstraction --- .../ApplicationHost.cs | 11 +- .../Diagnostics/CommonProcess.cs | 152 ------------------ .../Diagnostics/ProcessFactory.cs | 5 +- .../LiveTv/EmbyTV/EmbyTV.cs | 18 ++- .../LiveTv/EmbyTV/EncodedRecorder.cs | 23 ++- .../Extensions/ProcessExtensions.cs | 66 ++++++++ .../Encoder/MediaEncoder.cs | 31 ++-- .../Subtitles/SubtitleEncoder.cs | 13 +- MediaBrowser.Model/Diagnostics/IProcess.cs | 23 --- .../Diagnostics/IProcessFactory.cs | 19 +-- 10 files changed, 120 insertions(+), 241 deletions(-) delete mode 100644 Emby.Server.Implementations/Diagnostics/CommonProcess.cs create mode 100644 MediaBrowser.Common/Extensions/ProcessExtensions.cs delete mode 100644 MediaBrowser.Model/Diagnostics/IProcess.cs diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs index d6a572818a..bc637b02f1 100644 --- a/Emby.Server.Implementations/ApplicationHost.cs +++ b/Emby.Server.Implementations/ApplicationHost.cs @@ -1727,15 +1727,15 @@ namespace Emby.Server.Implementations throw new NotSupportedException(); } - var process = ProcessFactory.Create(new ProcessOptions + var process = ProcessFactory.Create(new ProcessStartInfo { FileName = url, - EnableRaisingEvents = true, UseShellExecute = true, ErrorDialog = false }); - process.Exited += ProcessExited; + process.EnableRaisingEvents = true; + process.Exited += (sender, args) => ((Process)sender).Dispose(); ; try { @@ -1748,11 +1748,6 @@ namespace Emby.Server.Implementations } } - private static void ProcessExited(object sender, EventArgs e) - { - ((IProcess)sender).Dispose(); - } - public virtual void EnableLoopback(string appName) { } diff --git a/Emby.Server.Implementations/Diagnostics/CommonProcess.cs b/Emby.Server.Implementations/Diagnostics/CommonProcess.cs deleted file mode 100644 index bfa49ac5ff..0000000000 --- a/Emby.Server.Implementations/Diagnostics/CommonProcess.cs +++ /dev/null @@ -1,152 +0,0 @@ -#pragma warning disable CS1591 - -using System; -using System.Diagnostics; -using System.IO; -using System.Threading; -using System.Threading.Tasks; -using MediaBrowser.Model.Diagnostics; - -namespace Emby.Server.Implementations.Diagnostics -{ - public class CommonProcess : IProcess - { - private readonly Process _process; - - private bool _disposed = false; - private bool _hasExited; - - public CommonProcess(ProcessOptions options) - { - StartInfo = options; - - var startInfo = new ProcessStartInfo - { - Arguments = options.Arguments, - FileName = options.FileName, - WorkingDirectory = options.WorkingDirectory, - UseShellExecute = options.UseShellExecute, - CreateNoWindow = options.CreateNoWindow, - RedirectStandardError = options.RedirectStandardError, - RedirectStandardInput = options.RedirectStandardInput, - RedirectStandardOutput = options.RedirectStandardOutput, - ErrorDialog = options.ErrorDialog - }; - - - if (options.IsHidden) - { - startInfo.WindowStyle = ProcessWindowStyle.Hidden; - } - - _process = new Process - { - StartInfo = startInfo - }; - - if (options.EnableRaisingEvents) - { - _process.EnableRaisingEvents = true; - _process.Exited += OnProcessExited; - } - } - - public event EventHandler Exited; - - public ProcessOptions StartInfo { get; } - - public StreamWriter StandardInput => _process.StandardInput; - - public StreamReader StandardError => _process.StandardError; - - public StreamReader StandardOutput => _process.StandardOutput; - - public int ExitCode => _process.ExitCode; - - private bool HasExited - { - get - { - if (_hasExited) - { - return true; - } - - try - { - _hasExited = _process.HasExited; - } - catch (InvalidOperationException) - { - _hasExited = true; - } - - return _hasExited; - } - } - - public void Start() - { - _process.Start(); - } - - public void Kill() - { - _process.Kill(); - } - - public bool WaitForExit(int timeMs) - { - return _process.WaitForExit(timeMs); - } - - public Task WaitForExitAsync(int timeMs) - { - // Note: For this function to work correctly, the option EnableRisingEvents needs to be set to true. - - if (HasExited) - { - return Task.FromResult(true); - } - - timeMs = Math.Max(0, timeMs); - - var tcs = new TaskCompletionSource(); - - var cancellationToken = new CancellationTokenSource(timeMs).Token; - - _process.Exited += (sender, args) => tcs.TrySetResult(true); - - cancellationToken.Register(() => tcs.TrySetResult(HasExited)); - - return tcs.Task; - } - - public void Dispose() - { - Dispose(true); - GC.SuppressFinalize(this); - } - - protected virtual void Dispose(bool disposing) - { - if (_disposed) - { - return; - } - - if (disposing) - { - _process?.Dispose(); - } - - _disposed = true; - } - - private void OnProcessExited(object sender, EventArgs e) - { - _hasExited = true; - Exited?.Invoke(this, e); - } - } -} diff --git a/Emby.Server.Implementations/Diagnostics/ProcessFactory.cs b/Emby.Server.Implementations/Diagnostics/ProcessFactory.cs index 02ad3c1a89..00172e17a6 100644 --- a/Emby.Server.Implementations/Diagnostics/ProcessFactory.cs +++ b/Emby.Server.Implementations/Diagnostics/ProcessFactory.cs @@ -1,14 +1,15 @@ #pragma warning disable CS1591 +using System.Diagnostics; using MediaBrowser.Model.Diagnostics; namespace Emby.Server.Implementations.Diagnostics { public class ProcessFactory : IProcessFactory { - public IProcess Create(ProcessOptions options) + public Process Create(ProcessStartInfo startInfo) { - return new CommonProcess(options); + return new Process { StartInfo = startInfo }; } } } diff --git a/Emby.Server.Implementations/LiveTv/EmbyTV/EmbyTV.cs b/Emby.Server.Implementations/LiveTv/EmbyTV/EmbyTV.cs index 139aa19a4b..0f54022c8e 100644 --- a/Emby.Server.Implementations/LiveTv/EmbyTV/EmbyTV.cs +++ b/Emby.Server.Implementations/LiveTv/EmbyTV/EmbyTV.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Diagnostics; using System.Globalization; using System.IO; using System.Linq; @@ -1683,19 +1684,19 @@ namespace Emby.Server.Implementations.LiveTv.EmbyTV try { - var process = _processFactory.Create(new ProcessOptions + var process = _processFactory.Create(new ProcessStartInfo { Arguments = GetPostProcessArguments(path, options.RecordingPostProcessorArguments), CreateNoWindow = true, - EnableRaisingEvents = true, ErrorDialog = false, FileName = options.RecordingPostProcessor, - IsHidden = true, + WindowStyle = ProcessWindowStyle.Hidden, UseShellExecute = false }); _logger.LogInformation("Running recording post processor {0} {1}", process.StartInfo.FileName, process.StartInfo.Arguments); + process.EnableRaisingEvents = true; process.Exited += Process_Exited; process.Start(); } @@ -1712,11 +1713,14 @@ namespace Emby.Server.Implementations.LiveTv.EmbyTV private void Process_Exited(object sender, EventArgs e) { - using (var process = (IProcess)sender) + try { - _logger.LogInformation("Recording post-processing script completed with exit code {ExitCode}", process.ExitCode); - - process.Dispose(); + var exitCode = ((Process)sender).ExitCode; + _logger.LogInformation("Recording post-processing script completed with exit code {ExitCode}", exitCode); + } + finally + { + ((Process)sender).Dispose(); } } diff --git a/Emby.Server.Implementations/LiveTv/EmbyTV/EncodedRecorder.cs b/Emby.Server.Implementations/LiveTv/EmbyTV/EncodedRecorder.cs index 8590c56dfd..3591384de1 100644 --- a/Emby.Server.Implementations/LiveTv/EmbyTV/EncodedRecorder.cs +++ b/Emby.Server.Implementations/LiveTv/EmbyTV/EncodedRecorder.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Globalization; using System.IO; using System.Text; @@ -30,7 +31,7 @@ namespace Emby.Server.Implementations.LiveTv.EmbyTV private bool _hasExited; private Stream _logFileStream; private string _targetPath; - private IProcess _process; + private Process _process; private readonly IProcessFactory _processFactory; private readonly IJsonSerializer _json; private readonly TaskCompletionSource _taskCompletionSource = new TaskCompletionSource(); @@ -80,7 +81,7 @@ namespace Emby.Server.Implementations.LiveTv.EmbyTV _targetPath = targetFile; Directory.CreateDirectory(Path.GetDirectoryName(targetFile)); - var process = _processFactory.Create(new ProcessOptions + _process = _processFactory.Create(new ProcessStartInfo { CreateNoWindow = true, UseShellExecute = false, @@ -91,14 +92,11 @@ namespace Emby.Server.Implementations.LiveTv.EmbyTV FileName = _mediaEncoder.EncoderPath, Arguments = GetCommandLineArgs(mediaSource, inputFile, targetFile, duration), - IsHidden = true, - ErrorDialog = false, - EnableRaisingEvents = true + WindowStyle = ProcessWindowStyle.Hidden, + ErrorDialog = false }); - _process = process; - - var commandLineLogMessage = process.StartInfo.FileName + " " + process.StartInfo.Arguments; + var commandLineLogMessage = _process.StartInfo.FileName + " " + _process.StartInfo.Arguments; _logger.LogInformation(commandLineLogMessage); var logFilePath = Path.Combine(_appPaths.LogDirectoryPath, "record-transcode-" + Guid.NewGuid() + ".txt"); @@ -110,16 +108,17 @@ namespace Emby.Server.Implementations.LiveTv.EmbyTV var commandLineLogMessageBytes = Encoding.UTF8.GetBytes(_json.SerializeToString(mediaSource) + Environment.NewLine + Environment.NewLine + commandLineLogMessage + Environment.NewLine + Environment.NewLine); _logFileStream.Write(commandLineLogMessageBytes, 0, commandLineLogMessageBytes.Length); - process.Exited += (sender, args) => OnFfMpegProcessExited(process, inputFile); + _process.EnableRaisingEvents = true; + _process.Exited += (sender, args) => OnFfMpegProcessExited(_process, inputFile); - process.Start(); + _process.Start(); cancellationToken.Register(Stop); onStarted(); // Important - don't await the log task or we won't be able to kill ffmpeg when the user stops playback - StartStreamingLog(process.StandardError.BaseStream, _logFileStream); + StartStreamingLog(_process.StandardError.BaseStream, _logFileStream); _logger.LogInformation("ffmpeg recording process started for {0}", _targetPath); @@ -293,7 +292,7 @@ namespace Emby.Server.Implementations.LiveTv.EmbyTV /// /// Processes the exited. /// - private void OnFfMpegProcessExited(IProcess process, string inputFile) + private void OnFfMpegProcessExited(Process process, string inputFile) { _hasExited = true; diff --git a/MediaBrowser.Common/Extensions/ProcessExtensions.cs b/MediaBrowser.Common/Extensions/ProcessExtensions.cs new file mode 100644 index 0000000000..9fa0efdff8 --- /dev/null +++ b/MediaBrowser.Common/Extensions/ProcessExtensions.cs @@ -0,0 +1,66 @@ +using System; +using System.Diagnostics; +using System.Threading; +using System.Threading.Tasks; + +namespace MediaBrowser.Common.Extensions +{ + /// + /// Extension methods for . + /// + public static class ProcessExtensions + { + /// + /// Gets a value indicating whether the associated process has been terminated using + /// . This is safe to call even if there is no operating system process + /// associated with the . + /// + /// The process to check the exit status for. + /// + /// True if the operating system process referenced by the component has + /// terminated, or if there is no associated operating system process; otherwise, false. + /// + public static bool HasExitedSafe(this Process process) + { + try + { + return process.HasExited; + } + catch (InvalidOperationException) + { + return true; + } + } + + /// + /// Asynchronously wait for the process to exit. + /// + /// The process to wait for. + /// A timeout, in milliseconds, after which to stop waiting for the task. + /// True if the task exited normally, false if the timeout elapsed before the process exited. + public static async Task WaitForExitAsync(this Process process, int timeMs) + { + if (!process.EnableRaisingEvents) + { + throw new InvalidOperationException("EnableRisingEvents must be enabled to async wait for a task to exit."); + } + + // Add an event handler for the process exit event + var tcs = new TaskCompletionSource(); + process.Exited += (sender, args) => tcs.TrySetResult(true); + + // Return immediately if the process has already exited + if (process.HasExitedSafe()) + { + return true; + } + + // Add an additional timeout then await + using (var cancelTokenSource = new CancellationTokenSource(Math.Max(0, timeMs))) + using (var cancelRegistration = cancelTokenSource.Token.Register(() => tcs.TrySetResult(process.HasExitedSafe()))) + { + return await tcs.Task.ConfigureAwait(false); + } + } + } +} diff --git a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs index 4123f0203a..dbb7dea073 100644 --- a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs +++ b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs @@ -13,7 +13,6 @@ using MediaBrowser.Controller.Configuration; using MediaBrowser.Controller.MediaEncoding; using MediaBrowser.MediaEncoding.Probing; using MediaBrowser.Model.Configuration; -using MediaBrowser.Model.Diagnostics; using MediaBrowser.Model.Dlna; using MediaBrowser.Model.Entities; using MediaBrowser.Model.Globalization; @@ -22,6 +21,8 @@ using MediaBrowser.Model.MediaInfo; using MediaBrowser.Model.System; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Configuration; +using System.Diagnostics; +using MediaBrowser.Model.Diagnostics; namespace MediaBrowser.MediaEncoding.Encoder { @@ -362,7 +363,7 @@ namespace MediaBrowser.MediaEncoding.Encoder : "{0} -i {1} -threads 0 -v warning -print_format json -show_streams -show_format"; args = string.Format(args, probeSizeArgument, inputPath).Trim(); - var process = _processFactory.Create(new ProcessOptions + var process = _processFactory.Create(new ProcessStartInfo { CreateNoWindow = true, UseShellExecute = false, @@ -374,10 +375,10 @@ namespace MediaBrowser.MediaEncoding.Encoder Arguments = args, - IsHidden = true, + WindowStyle = ProcessWindowStyle.Hidden, ErrorDialog = false, - EnableRaisingEvents = true }); + process.EnableRaisingEvents = true; if (forceEnableLogging) { @@ -571,16 +572,16 @@ namespace MediaBrowser.MediaEncoding.Encoder } } - var process = _processFactory.Create(new ProcessOptions + var process = _processFactory.Create(new ProcessStartInfo { CreateNoWindow = true, UseShellExecute = false, FileName = _ffmpegPath, Arguments = args, - IsHidden = true, + WindowStyle = ProcessWindowStyle.Hidden, ErrorDialog = false, - EnableRaisingEvents = true }); + process.EnableRaisingEvents = true; _logger.LogDebug("{0} {1}", process.StartInfo.FileName, process.StartInfo.Arguments); @@ -700,16 +701,16 @@ namespace MediaBrowser.MediaEncoding.Encoder } } - var process = _processFactory.Create(new ProcessOptions + var process = _processFactory.Create(new ProcessStartInfo { CreateNoWindow = true, UseShellExecute = false, FileName = _ffmpegPath, Arguments = args, - IsHidden = true, - ErrorDialog = false, - EnableRaisingEvents = true + WindowStyle = ProcessWindowStyle.Hidden, + ErrorDialog = false }); + process.EnableRaisingEvents = true; _logger.LogInformation(process.StartInfo.FileName + " " + process.StartInfo.Arguments); @@ -949,14 +950,14 @@ namespace MediaBrowser.MediaEncoding.Encoder private bool _disposed = false; - public ProcessWrapper(IProcess process, MediaEncoder mediaEncoder) + public ProcessWrapper(Process process, MediaEncoder mediaEncoder) { Process = process; _mediaEncoder = mediaEncoder; Process.Exited += OnProcessExited; } - public IProcess Process { get; } + public Process Process { get; } public bool HasExited { get; private set; } @@ -964,7 +965,7 @@ namespace MediaBrowser.MediaEncoding.Encoder void OnProcessExited(object sender, EventArgs e) { - var process = (IProcess)sender; + var process = (Process)sender; HasExited = true; @@ -979,7 +980,7 @@ namespace MediaBrowser.MediaEncoding.Encoder DisposeProcess(process); } - private void DisposeProcess(IProcess process) + private void DisposeProcess(Process process) { lock (_mediaEncoder._runningProcessesLock) { diff --git a/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs b/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs index 6284e0fd9e..50ff834c62 100644 --- a/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs +++ b/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Concurrent; +using System.Diagnostics; using System.Globalization; using System.IO; using System.Linq; @@ -429,14 +430,13 @@ namespace MediaBrowser.MediaEncoding.Subtitles encodingParam = " -sub_charenc " + encodingParam; } - var process = _processFactory.Create(new ProcessOptions + var process = _processFactory.Create(new ProcessStartInfo { CreateNoWindow = true, UseShellExecute = false, FileName = _mediaEncoder.EncoderPath, Arguments = string.Format("{0} -i \"{1}\" -c:s srt \"{2}\"", encodingParam, inputPath, outputPath), - EnableRaisingEvents = true, - IsHidden = true, + WindowStyle = ProcessWindowStyle.Hidden, ErrorDialog = false }); @@ -453,6 +453,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles throw; } + process.EnableRaisingEvents = true; var ranToCompletion = await process.WaitForExitAsync(300000).ConfigureAwait(false); if (!ranToCompletion) @@ -578,14 +579,13 @@ namespace MediaBrowser.MediaEncoding.Subtitles outputCodec, outputPath); - var process = _processFactory.Create(new ProcessOptions + var process = _processFactory.Create(new ProcessStartInfo { CreateNoWindow = true, UseShellExecute = false, - EnableRaisingEvents = true, FileName = _mediaEncoder.EncoderPath, Arguments = processArgs, - IsHidden = true, + WindowStyle = ProcessWindowStyle.Hidden, ErrorDialog = false }); @@ -602,6 +602,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles throw; } + process.EnableRaisingEvents = true; var ranToCompletion = await process.WaitForExitAsync(300000).ConfigureAwait(false); if (!ranToCompletion) diff --git a/MediaBrowser.Model/Diagnostics/IProcess.cs b/MediaBrowser.Model/Diagnostics/IProcess.cs deleted file mode 100644 index 514d1e7379..0000000000 --- a/MediaBrowser.Model/Diagnostics/IProcess.cs +++ /dev/null @@ -1,23 +0,0 @@ -#pragma warning disable CS1591 - -using System; -using System.IO; -using System.Threading.Tasks; - -namespace MediaBrowser.Model.Diagnostics -{ - public interface IProcess : IDisposable - { - event EventHandler Exited; - - void Kill(); - bool WaitForExit(int timeMs); - Task WaitForExitAsync(int timeMs); - int ExitCode { get; } - void Start(); - StreamWriter StandardInput { get; } - StreamReader StandardError { get; } - StreamReader StandardOutput { get; } - ProcessOptions StartInfo { get; } - } -} diff --git a/MediaBrowser.Model/Diagnostics/IProcessFactory.cs b/MediaBrowser.Model/Diagnostics/IProcessFactory.cs index 57082acc57..d95227797c 100644 --- a/MediaBrowser.Model/Diagnostics/IProcessFactory.cs +++ b/MediaBrowser.Model/Diagnostics/IProcessFactory.cs @@ -1,24 +1,11 @@ #pragma warning disable CS1591 +using System.Diagnostics; + namespace MediaBrowser.Model.Diagnostics { public interface IProcessFactory { - IProcess Create(ProcessOptions options); - } - - public class ProcessOptions - { - public string FileName { get; set; } - public string Arguments { get; set; } - public string WorkingDirectory { get; set; } - public bool CreateNoWindow { get; set; } - public bool UseShellExecute { get; set; } - public bool EnableRaisingEvents { get; set; } - public bool ErrorDialog { get; set; } - public bool RedirectStandardError { get; set; } - public bool RedirectStandardInput { get; set; } - public bool RedirectStandardOutput { get; set; } - public bool IsHidden { get; set; } + Process Create(ProcessStartInfo options); } }