From af8ec61559ade875bcf840251069a1eced7d035a Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 30 Apr 2019 12:11:40 +0200 Subject: [PATCH 1/5] Linux: ProcessName: return script name instead of interpreter program Fixes https://github.com/dotnet/corefx/issues/37198 Regression by https://github.com/dotnet/corefx/pull/37144 --- .../Linux/procfs/Interop.ProcFsStat.cs | 4 +- src/Common/src/System/IO/StringParser.cs | 4 +- src/Common/tests/Tests/Interop/procfsTests.cs | 1 + .../src/System/Diagnostics/Process.Linux.cs | 62 ++++++++++++++----- .../Diagnostics/ProcessManager.Linux.cs | 8 +-- .../tests/ProcessTests.Unix.cs | 1 - 6 files changed, 55 insertions(+), 25 deletions(-) diff --git a/src/Common/src/Interop/Linux/procfs/Interop.ProcFsStat.cs b/src/Common/src/Interop/Linux/procfs/Interop.ProcFsStat.cs index 66637a3a1457..cf9ab862c6cd 100644 --- a/src/Common/src/Interop/Linux/procfs/Interop.ProcFsStat.cs +++ b/src/Common/src/Interop/Linux/procfs/Interop.ProcFsStat.cs @@ -33,7 +33,7 @@ internal struct ParsedStat // the MoveNext() with the appropriate ParseNext* call and assignment. internal int pid; - // internal string comm; + internal string comm; internal char state; internal int ppid; //internal int pgrp; @@ -238,7 +238,7 @@ internal static bool TryParseStatFile(string statFilePath, out ParsedStat result var results = default(ParsedStat); results.pid = parser.ParseNextInt32(); - parser.MoveAndExtractNextInOuterParens(extractValue: false); // comm + results.comm = parser.MoveAndExtractNextInOuterParens(); results.state = parser.ParseNextChar(); results.ppid = parser.ParseNextInt32(); parser.MoveNextOrFail(); // pgrp diff --git a/src/Common/src/System/IO/StringParser.cs b/src/Common/src/System/IO/StringParser.cs index b94a7000d876..d79d7ce2e82e 100644 --- a/src/Common/src/System/IO/StringParser.cs +++ b/src/Common/src/System/IO/StringParser.cs @@ -98,7 +98,7 @@ public string MoveAndExtractNext() /// in the string. The extracted value will be everything between (not including) those parentheses. /// /// - public string MoveAndExtractNextInOuterParens(bool extractValue = true) + public string MoveAndExtractNextInOuterParens() { // Move to the next position MoveNextOrFail(); @@ -118,7 +118,7 @@ public string MoveAndExtractNextInOuterParens(bool extractValue = true) } // Extract the contents of the parens, then move our ending position to be after the paren - string result = extractValue ? _buffer.Substring(_startIndex + 1, lastParen - _startIndex - 1) : null; + string result = _buffer.Substring(_startIndex + 1, lastParen - _startIndex - 1); _endIndex = lastParen + 1; return result; diff --git a/src/Common/tests/Tests/Interop/procfsTests.cs b/src/Common/tests/Tests/Interop/procfsTests.cs index 7fd7647410fd..53aa6ae8d421 100644 --- a/src/Common/tests/Tests/Interop/procfsTests.cs +++ b/src/Common/tests/Tests/Interop/procfsTests.cs @@ -49,6 +49,7 @@ public static void ParseValidStatFiles_Success( Assert.True(Interop.procfs.TryParseStatFile(path, out result, new ReusableTextReader())); Assert.Equal(expectedPid, result.pid); + Assert.Equal(expectedComm, result.comm); Assert.Equal(expectedState, result.state); Assert.Equal(expectedSession, result.session); Assert.Equal(expectedUtime, result.utime); diff --git a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs b/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs index 013418acf92d..e6266efdc794 100644 --- a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs +++ b/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs @@ -30,9 +30,11 @@ public static Process[] GetProcessesByName(string processName, string machineNam var processes = new List(); foreach (int pid in ProcessManager.EnumerateProcessIds()) { - if (string.Equals(processName, Process.GetProcessName(pid), StringComparison.OrdinalIgnoreCase)) + Interop.procfs.ParsedStat parsedStat; + if (Interop.procfs.TryReadStatFile(pid, out parsedStat, reusableReader) && + string.Equals(processName, Process.GetUntruncatedProcessName(ref parsedStat), StringComparison.OrdinalIgnoreCase)) { - ProcessInfo processInfo = ProcessManager.CreateProcessInfo(pid, reusableReader, processName); + ProcessInfo processInfo = ProcessManager.CreateProcessInfo(ref parsedStat, reusableReader, processName); processes.Add(new Process(machineName, false, processInfo.ProcessId, processInfo)); } } @@ -256,12 +258,10 @@ internal static string GetExePath(int processId = -1) } /// Gets the name that was used to start the process, or null if it could not be retrieved. - /// The pid for the target process, or -1 for the current process. - internal static string GetProcessName(int processId = -1) + /// The stat for the target process. + internal static string GetUntruncatedProcessName(ref Interop.procfs.ParsedStat stat) { - string cmdLineFilePath = processId == -1 ? - Interop.procfs.SelfCmdLineFilePath : - Interop.procfs.GetCmdLinePathForProcess(processId); + string cmdLineFilePath = Interop.procfs.GetCmdLinePathForProcess(stat.pid); byte[] rentedArray = null; try @@ -282,7 +282,7 @@ internal static string GetProcessName(int processId = -1) buffer.CopyTo(tmp); byte[] toReturn = rentedArray; buffer = rentedArray = tmp; - if (rentedArray != null) + if (toReturn != null) { ArrayPool.Shared.Return(toReturn); } @@ -293,26 +293,40 @@ internal static string GetProcessName(int processId = -1) bytesRead += n; // cmdline contains the argv array separated by '\0' bytes. - // we determine the process name using argv[0]. - int argv0End = buffer.Slice(0, bytesRead).IndexOf((byte)'\0'); - if (argv0End != -1) + // stat.comm contains a possibly truncated version of the process name. + // When the program is a native executable, the process name will be in argv[0]. + // When the program is a script, argv[0] contains the interpreter, and argv[1] contains the script name. + Span argRemainder = buffer.Slice(0, bytesRead); + int argEnd = argRemainder.IndexOf((byte)'\0'); + if (argEnd != -1) { - // Strip directory names from argv[0]. - int nameStart = buffer.Slice(0, argv0End).LastIndexOf((byte)'/') + 1; + // Check if argv[0] has the process name. + string name = GetUntruncatedNameFromArg(argRemainder.Slice(0, argEnd), prefix: stat.comm); + if (name != null) + { + return name; + } - return Encoding.UTF8.GetString(buffer.Slice(nameStart, argv0End - nameStart)); + // Check if argv[1] has the process name. + argRemainder = argRemainder.Slice(argEnd + 1); + argEnd = argRemainder.IndexOf((byte)'\0'); + if (argEnd != -1) + { + name = GetUntruncatedNameFromArg(argRemainder.Slice(0, argEnd), prefix: stat.comm); + return name ?? stat.comm; + } } if (n == 0) { - return null; + return stat.comm; } } } } catch (IOException) { - return null; + return stat.comm; } finally { @@ -321,6 +335,22 @@ internal static string GetProcessName(int processId = -1) ArrayPool.Shared.Return(rentedArray); } } + + string GetUntruncatedNameFromArg(Span arg, string prefix) + { + // Strip directory names from arg. + int nameStart = arg.LastIndexOf((byte)'/') + 1; + string argString = Encoding.UTF8.GetString(arg.Slice(nameStart)); + + if (argString.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) + { + return argString; + } + else + { + return null; + } + } } // ---------------------------------- diff --git a/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Linux.cs b/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Linux.cs index 53e2c4586f97..41c5efbbacfb 100644 --- a/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Linux.cs +++ b/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Linux.cs @@ -108,7 +108,7 @@ internal static ProcessModuleCollection GetModules(int processId) /// /// Creates a ProcessInfo from the specified process ID. /// - internal static ProcessInfo CreateProcessInfo(int pid, ReusableTextReader reusableReader = null, string processName = null) + internal static ProcessInfo CreateProcessInfo(int pid, ReusableTextReader reusableReader = null) { if (reusableReader == null) { @@ -117,21 +117,21 @@ internal static ProcessInfo CreateProcessInfo(int pid, ReusableTextReader reusab Interop.procfs.ParsedStat stat; return Interop.procfs.TryReadStatFile(pid, out stat, reusableReader) ? - CreateProcessInfo(stat, reusableReader, processName) : + CreateProcessInfo(ref stat, reusableReader) : null; } /// /// Creates a ProcessInfo from the data parsed from a /proc/pid/stat file and the associated tasks directory. /// - internal static ProcessInfo CreateProcessInfo(Interop.procfs.ParsedStat procFsStat, ReusableTextReader reusableReader, string processName) + internal static ProcessInfo CreateProcessInfo(ref Interop.procfs.ParsedStat procFsStat, ReusableTextReader reusableReader, string processName = null) { int pid = procFsStat.pid; var pi = new ProcessInfo() { ProcessId = pid, - ProcessName = processName ?? Process.GetProcessName(pid) ?? string.Empty, + ProcessName = processName ?? Process.GetUntruncatedProcessName(ref procFsStat) ?? string.Empty, BasePriority = (int)procFsStat.nice, VirtualBytes = (long)procFsStat.vsize, WorkingSet = procFsStat.rss * Environment.SystemPageSize, diff --git a/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs index 8242d97bf330..94de927dcb47 100644 --- a/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs +++ b/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs @@ -124,7 +124,6 @@ public void ProcessStart_UseShellExecute_OnUnix_OpenMissingFile_DoesNotThrow() } } - [ActiveIssue(37198)] [Theory, InlineData(true), InlineData(false)] [OuterLoop("Opens program")] public void ProcessStart_UseShellExecute_OnUnix_SuccessWhenProgramInstalled(bool isFolder) From 07835dabe7acb20252b8c5eff0a60df014ce536f Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 30 Apr 2019 14:06:23 +0200 Subject: [PATCH 2/5] Add ProcessNameMatchesScriptName test --- .../tests/ProcessTests.Unix.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs index 94de927dcb47..3ca25136e823 100644 --- a/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs +++ b/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs @@ -156,6 +156,30 @@ public void ProcessStart_UseShellExecute_OnUnix_SuccessWhenProgramInstalled(bool } } + [Fact] + public void ProcessNameMatchesScriptName() + { + string scriptName = GetTestFileName(); + string filename = Path.Combine(TestDirectory, scriptName); + File.WriteAllText(filename, $"#!/bin/sh\nsleep 600\n"); // sleep 10 min. + // set x-bit + int mode = Convert.ToInt32("744", 8); + Assert.Equal(0, chmod(filename, mode)); + + using (var process = Process.Start(new ProcessStartInfo { FileName = filename })) + { + try + { + Assert.Equal(scriptName, process.ProcessName); + } + finally + { + process.Kill(); + process.WaitForExit(); + } + } + } + [Fact] [PlatformSpecific(TestPlatforms.Linux)] // s_allowedProgramsToRun is Linux specific public void ProcessStart_UseShellExecute_OnUnix_FallsBackWhenNotRealExecutable() From c5077ca65ebc41fcdea21c0bdfa64f6d9c430d9b Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 30 Apr 2019 16:29:39 +0200 Subject: [PATCH 3/5] ProcessNameMatchesScriptName: don't run on OSX --- src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs index 3ca25136e823..98e9a3edc7c9 100644 --- a/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs +++ b/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs @@ -157,6 +157,7 @@ public void ProcessStart_UseShellExecute_OnUnix_SuccessWhenProgramInstalled(bool } [Fact] + [PlatformSpecific(~TestPlatforms.OSX)] // On OSX, ProcessName returns the script interpreter. public void ProcessNameMatchesScriptName() { string scriptName = GetTestFileName(); From 19c8f2b8dcf319f01be8eb1616f87a527e56b253 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 30 Apr 2019 16:43:47 +0200 Subject: [PATCH 4/5] LongProcessNamesAreSupported: don't run on Alpine --- src/System.Diagnostics.Process/tests/ProcessTests.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/System.Diagnostics.Process/tests/ProcessTests.cs b/src/System.Diagnostics.Process/tests/ProcessTests.cs index 191830beb5c2..f55e2df4ae78 100644 --- a/src/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/System.Diagnostics.Process/tests/ProcessTests.cs @@ -1880,6 +1880,13 @@ public void TestLongProcessIsWorking() [Fact] public void LongProcessNamesAreSupported() { + // Alpine implements sleep as a symlink to the busybox executable. + // If we rename it, the program will no longer sleep. + if (PlatformDetection.IsAlpine) + { + return; + } + string programPath = GetProgramPath("sleep"); if (programPath == null) From a34dfa1058ba66590b9616fbbf642430d25c9087 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 10 May 2019 05:17:47 +0200 Subject: [PATCH 5/5] Remove ActiveIssue attributes --- src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs | 1 - src/System.Diagnostics.Process/tests/ProcessTests.cs | 1 - 2 files changed, 2 deletions(-) diff --git a/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs index f616f30ea2f6..98e9a3edc7c9 100644 --- a/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs +++ b/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs @@ -318,7 +318,6 @@ public void ProcessStart_OpenFileOnLinux_UsesSpecifiedProgram(string programToOp } } - [ActiveIssue(37198)] [Theory, InlineData("vi")] [PlatformSpecific(TestPlatforms.Linux)] [OuterLoop("Opens program")] diff --git a/src/System.Diagnostics.Process/tests/ProcessTests.cs b/src/System.Diagnostics.Process/tests/ProcessTests.cs index dbab752f6894..f85df07c8451 100644 --- a/src/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/System.Diagnostics.Process/tests/ProcessTests.cs @@ -1892,7 +1892,6 @@ public void TestLongProcessIsWorking() Assert.True(p.HasExited); } - [ActiveIssue(37198)] [PlatformSpecific(TestPlatforms.AnyUnix)] [ActiveIssue(37054, TestPlatforms.OSX)] [Fact]