Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Process.Unix: handle ETXTBSY error when executing file that was just written by .NET. #59079

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ internal static void ConfigureTerminalForChildProcesses(int increment, bool conf
int childrenUsingTerminalRemaining = Interlocked.Add(ref s_childrenUsingTerminalCount, increment);
if (increment > 0)
{
Debug.Assert(s_processStartLock.IsReadLockHeld);
Debug.Assert(s_processStartLock.IsUpgradeableReadLockHeld);
Debug.Assert(configureConsole);

// At least one child is using the terminal.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -507,54 +507,76 @@ private bool ForkAndExecProcess(

// Lock to avoid races with OnSigChild
// By using a ReaderWriterLock we allow multiple processes to start concurrently.
s_processStartLock.EnterReadLock();
s_processStartLock.EnterUpgradeableReadLock();
try
{
if (usesTerminal)
{
ConfigureTerminalForChildProcesses(1);
}

int childPid;

// Invoke the shim fork/execve routine. It will create pipes for all requested
// redirects, fork a child process, map the pipe ends onto the appropriate stdin/stdout/stderr
// descriptors, and execve to execute the requested process. The shim implementation
// is used to fork/execve as executing managed code in a forked process is not safe (only
// the calling thread will transfer, thread IDs aren't stable across the fork, etc.)
int errno = Interop.Sys.ForkAndExecProcess(
resolvedFilename, argv, envp, cwd,
startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError,
setCredentials, userId, groupId, groups,
out childPid, out stdinFd, out stdoutFd, out stderrFd);

if (errno == 0)
bool retryOnTxtBsy = true;
do
{
// Ensure we'll reap this process.
// note: SetProcessId will set this if we don't set it first.
_waitStateHolder = new ProcessWaitState.Holder(childPid, isNewChild: true, usesTerminal);

// Store the child's information into this Process object.
Debug.Assert(childPid >= 0);
SetProcessId(childPid);
SetProcessHandle(new SafeProcessHandle(_processId, GetSafeWaitHandle()));
int childPid;

// Invoke the shim fork/execve routine. It will create pipes for all requested
// redirects, fork a child process, map the pipe ends onto the appropriate stdin/stdout/stderr
// descriptors, and execve to execute the requested process. The shim implementation
// is used to fork/execve as executing managed code in a forked process is not safe (only
// the calling thread will transfer, thread IDs aren't stable across the fork, etc.)
int errno = Interop.Sys.ForkAndExecProcess(
resolvedFilename, argv, envp, cwd,
startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError,
setCredentials, userId, groupId, groups,
out childPid, out stdinFd, out stdoutFd, out stderrFd);

return true;
}
else
{
if (!throwOnNoExec &&
new Interop.ErrorInfo(errno).Error == Interop.Error.ENOEXEC)
if (errno == 0)
{
return false;
// Ensure we'll reap this process.
// note: SetProcessId will set this if we don't set it first.
_waitStateHolder = new ProcessWaitState.Holder(childPid, isNewChild: true, usesTerminal);

// Store the child's information into this Process object.
Debug.Assert(childPid >= 0);
SetProcessId(childPid);
SetProcessHandle(new SafeProcessHandle(_processId, GetSafeWaitHandle()));

return true;
}
else
{
Interop.ErrorInfo errorInfo = new Interop.ErrorInfo(errno);

throw CreateExceptionForErrorStartingProcess(new Interop.ErrorInfo(errno).GetErrorMessage(), errno, resolvedFilename, cwd);
}
if (!throwOnNoExec && errorInfo.Error == Interop.Error.ENOEXEC)
{
return false;
}

if (retryOnTxtBsy && errorInfo.Error == Interop.Error.ETXTBSY)
{
retryOnTxtBsy = false;

// ETXTBSY means we're trying to execute a file that is open for write.
// This may be a file that was just written and closed, but is still referenced
// by another process that has forked but not yet exec-ed.
// Using the s_processStartLock we wait for all processes to finish exec-ing.
// 'ForkAndExecProcess' returns when the a pipe gets closed by exec.
// Unfortunately, the handle we care about may be closed after that. So this
// doesn't work all the time.
s_processStartLock.EnterWriteLock();
s_processStartLock.ExitWriteLock();

continue;
}

throw CreateExceptionForErrorStartingProcess(errorInfo.GetErrorMessage(), errno, resolvedFilename, cwd);
}
} while (true);
}
finally
{
s_processStartLock.ExitReadLock();
s_processStartLock.ExitUpgradeableReadLock();

if (_waitStateHolder == null && usesTerminal)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,21 @@ public void TestStartOnUnixWithBadFormat()
Assert.NotEqual(0, e.NativeErrorCode);
}

[Fact]
[PlatformSpecific(TestPlatforms.Linux)] // OSX doesn't fail when executing a file that is open for writing.
public async Task ExecutingFileThatIsOpenForWritingDoesntHang()
{
// On Linux, a process fails to execute with 'ETXTBSY' when the executable is open for writing.
// Process.Unix has some special handling for that.
// This test verifies it doesn't cause 'Process.Start' to hang indefinitely when the handle remains open.

string scriptFile = WriteScriptFile(TestDirectory, GetTestFileName(), 42);
using var handle = File.OpenHandle(scriptFile, FileMode.Open, FileAccess.Write);

await Assert.ThrowsAsync<Win32Exception>(() =>
Task.Run(() => Process.Start(scriptFile)).WaitAsync(TimeSpan.FromSeconds(30)));
}

[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public void TestStartWithNonExistingUserThrows()
{
Expand Down Expand Up @@ -996,7 +1011,7 @@ private static unsafe HashSet<uint> GetGroups()

private static readonly string[] s_allowedProgramsToRun = new string[] { "xdg-open", "gnome-open", "kfmclient" };

private string WriteScriptFile(string directory, string name, int returnValue)
private static string WriteScriptFile(string directory, string name, int returnValue)
{
string filename = Path.Combine(directory, name);
File.WriteAllText(filename, $"#!/bin/sh\nexit {returnValue}\n");
Expand Down