Skip to content

Commit 7aaad5e

Browse files
CarterLiCarterLi
committed
Processing: refactors process execution to separate spawning and reading
1 parent 645bf5c commit 7aaad5e

File tree

3 files changed

+105
-54
lines changed

3 files changed

+105
-54
lines changed

‎src/common/processing.h‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,38 @@
22

33
#include "util/FFstrbuf.h"
44

5-
#include <sys/types.h>
5+
typedef struct FFProcessHandle {
6+
#if _WIN32
7+
void* pid; // HANDLE
8+
void* pipeRead; // HANDLE
9+
#else
10+
pid_t pid;
11+
int pipeRead;
12+
#endif
13+
} FFProcessHandle;
614

7-
const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool useStdErr);
15+
const char* ffProcessSpawn(char* const argv[], bool useStdErr, FFProcessHandle* outHandle);
16+
const char* ffProcessReadOutput(FFProcessHandle* handle, FFstrbuf* buffer); // Destroys handle internally
817

918
static inline const char* ffProcessAppendStdOut(FFstrbuf* buffer, char* const argv[])
1019
{
11-
const char* error = ffProcessAppendOutput(buffer, argv, false);
20+
FFProcessHandle handle;
21+
const char* error = ffProcessSpawn(argv, false, &handle);
22+
if (error) return error;
23+
24+
error = ffProcessReadOutput(&handle, buffer);
1225
if (!error)
1326
ffStrbufTrimRightSpace(buffer);
1427
return error;
1528
}
1629

1730
static inline const char* ffProcessAppendStdErr(FFstrbuf* buffer, char* const argv[])
1831
{
19-
const char* error = ffProcessAppendOutput(buffer, argv, true);
32+
FFProcessHandle handle;
33+
const char* error = ffProcessSpawn(argv, true, &handle);
34+
if (error) return error;
35+
36+
error = ffProcessReadOutput(&handle, buffer);
2037
if (!error)
2138
ffStrbufTrimRightSpace(buffer);
2239
return error;

‎src/common/processing_linux.c‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,9 @@ static inline int ffPipe2(int* fds, int flags)
5858

5959

6060
// Not thread-safe
61-
const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool useStdErr)
61+
const char* ffProcessSpawn(char* const argv[], bool useStdErr, FFProcessHandle* outHandle)
6262
{
6363
int pipes[2];
64-
const int timeout = instance.config.general.processingTimeout;
65-
6664
if(ffPipe2(pipes, O_CLOEXEC) == -1)
6765
return "pipe() failed";
6866

@@ -146,7 +144,7 @@ const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool use
146144

147145
if(childPid == 0)
148146
{
149-
//Child
147+
// Child process
150148
dup2(pipes[1], useStdErr ? STDERR_FILENO : STDOUT_FILENO);
151149
dup2(nullFile, useStdErr ? STDOUT_FILENO : STDERR_FILENO);
152150
putenv("LANG=C.UTF-8");
@@ -157,11 +155,24 @@ const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool use
157155
#endif
158156

159157
close(pipes[1]);
158+
outHandle->pid = childPid;
159+
outHandle->pipeRead = pipes[0];
160+
return NULL;
161+
}
160162

161-
int FF_AUTO_CLOSE_FD childPipeFd = pipes[0];
163+
const char* ffProcessReadOutput(FFProcessHandle* handle, FFstrbuf* buffer)
164+
{
165+
assert(handle->pipeRead != -1);
166+
assert(handle->pid != -1);
167+
168+
const int32_t timeout = instance.config.general.processingTimeout;
169+
FF_AUTO_CLOSE_FD int childPipeFd = handle->pipeRead;
170+
pid_t childPid = handle->pid;
171+
handle->pipeRead = -1;
172+
handle->pid = -1;
162173
char str[FF_PIPE_BUFSIZ];
163174

164-
while(1)
175+
for (;;)
165176
{
166177
if (timeout >= 0)
167178
{
@@ -189,14 +200,14 @@ const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool use
189200
{
190201
kill(childPid, SIGTERM);
191202
waitpid(childPid, NULL, 0);
192-
return "poll(&pollfd, 1, timeout) error";
203+
return "poll(&pollfd, 1, timeout) error: not EINTR";
193204
}
194205
}
195206
else if (pollfd.revents & POLLERR)
196207
{
197208
kill(childPid, SIGTERM);
198209
waitpid(childPid, NULL, 0);
199-
return "poll(&pollfd, 1, timeout) error";
210+
return "poll(&pollfd, 1, timeout) error: POLLERR";
200211
}
201212
}
202213

@@ -211,7 +222,7 @@ const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool use
211222
if (!WIFEXITED(stat_loc))
212223
return "child process exited abnormally";
213224
if (WEXITSTATUS(stat_loc) == 127)
214-
return "command was not found";
225+
return "command not found";
215226
// We only handle 127 as an error. See `getTerminalVersionUrxvt` in `terminalshell.c`
216227
return NULL;
217228
}
@@ -224,7 +235,7 @@ const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool use
224235
else
225236
break;
226237
}
227-
};
238+
}
228239

229240
return "read(childPipeFd, str, FF_PIPE_BUFSIZ) failed";
230241
}

‎src/common/processing_windows.c‎

Lines changed: 63 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,13 @@ static void argvToCmdline(char* const argv[], FFstrbuf* result)
4545
}
4646
}
4747

48-
const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool useStdErr)
48+
const char* ffProcessSpawn(char* const argv[], bool useStdErr, FFProcessHandle* outHandle)
4949
{
50-
int timeout = instance.config.general.processingTimeout;
50+
const int32_t timeout = instance.config.general.processingTimeout;
5151

5252
wchar_t pipeName[32];
53-
swprintf(pipeName, ARRAY_SIZE(pipeName), L"\\\\.\\pipe\\FASTFETCH-%u", GetCurrentProcessId());
53+
static unsigned pidCounter = 0;
54+
swprintf(pipeName, ARRAY_SIZE(pipeName), L"\\\\.\\pipe\\FASTFETCH-%u-%u", GetCurrentProcessId(), ++pidCounter);
5455

5556
FF_AUTO_CLOSE_FD HANDLE hChildPipeRead = CreateNamedPipeW(
5657
pipeName,
@@ -82,52 +83,67 @@ const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool use
8283
return "CreateFileW(L\"\\\\.\\pipe\\FASTFETCH-$(PID)\") failed";
8384

8485
PROCESS_INFORMATION piProcInfo = {};
85-
86-
BOOL success;
87-
86+
STARTUPINFOA siStartInfo = {
87+
.cb = sizeof(siStartInfo),
88+
.dwFlags = STARTF_USESTDHANDLES,
89+
};
90+
if (useStdErr)
8891
{
89-
STARTUPINFOA siStartInfo = {
90-
.cb = sizeof(siStartInfo),
91-
.dwFlags = STARTF_USESTDHANDLES,
92-
};
93-
if (useStdErr)
94-
{
95-
siStartInfo.hStdOutput = ffGetNullFD();
96-
siStartInfo.hStdError = hChildPipeWrite;
97-
}
98-
else
99-
{
100-
siStartInfo.hStdOutput = hChildPipeWrite;
101-
siStartInfo.hStdError = ffGetNullFD();
102-
}
103-
104-
FF_STRBUF_AUTO_DESTROY cmdline = ffStrbufCreate();
105-
argvToCmdline(argv, &cmdline);
106-
107-
success = CreateProcessA(
108-
NULL, // application name
109-
cmdline.chars, // command line
110-
NULL, // process security attributes
111-
NULL, // primary thread security attributes
112-
TRUE, // handles are inherited
113-
0, // creation flags
114-
NULL, // use parent's environment
115-
NULL, // use parent's current directory
116-
&siStartInfo, // STARTUPINFO pointer
117-
&piProcInfo // receives PROCESS_INFORMATION
118-
);
92+
siStartInfo.hStdOutput = ffGetNullFD();
93+
siStartInfo.hStdError = hChildPipeWrite;
94+
}
95+
else
96+
{
97+
siStartInfo.hStdOutput = hChildPipeWrite;
98+
siStartInfo.hStdError = ffGetNullFD();
11999
}
120100

101+
FF_STRBUF_AUTO_DESTROY cmdline = ffStrbufCreate();
102+
argvToCmdline(argv, &cmdline);
103+
104+
BOOL success = CreateProcessA(
105+
NULL, // application name
106+
cmdline.chars, // command line
107+
NULL, // process security attributes
108+
NULL, // primary thread security attributes
109+
TRUE, // handles are inherited
110+
0, // creation flags
111+
NULL, // use parent's environment
112+
NULL, // use parent's current directory
113+
&siStartInfo, // STARTUPINFO pointer
114+
&piProcInfo // receives PROCESS_INFORMATION
115+
);
116+
121117
CloseHandle(hChildPipeWrite);
122118
if(!success)
119+
{
120+
if (GetLastError() == ERROR_FILE_NOT_FOUND)
121+
return "command not found";
123122
return "CreateProcessA() failed";
123+
}
124+
125+
CloseHandle(piProcInfo.hThread); // we don't need the thread handle
126+
outHandle->pid = piProcInfo.hProcess;
127+
outHandle->pipeRead = hChildPipeRead;
128+
hChildPipeRead = INVALID_HANDLE_VALUE; // ownership transferred, don't close it
124129

125-
FF_AUTO_CLOSE_FD HANDLE hProcess = piProcInfo.hProcess;
126-
FF_MAYBE_UNUSED FF_AUTO_CLOSE_FD HANDLE hThread = piProcInfo.hThread;
130+
return NULL;
131+
}
132+
133+
const char* ffProcessReadOutput(FFProcessHandle* handle, FFstrbuf* buffer)
134+
{
135+
assert(handle->pipeRead != INVALID_HANDLE_VALUE);
136+
assert(handle->pid != INVALID_HANDLE_VALUE);
137+
138+
int32_t timeout = instance.config.general.processingTimeout;
139+
FF_AUTO_CLOSE_FD HANDLE hProcess = handle->pid;
140+
FF_AUTO_CLOSE_FD HANDLE hChildPipeRead = handle->pipeRead;
141+
handle->pid = INVALID_HANDLE_VALUE;
142+
handle->pipeRead = INVALID_HANDLE_VALUE;
127143

128144
char str[FF_PIPE_BUFSIZ];
129145
DWORD nRead = 0;
130-
OVERLAPPED overlapped = { };
146+
OVERLAPPED overlapped = {};
131147
// ReadFile always completes synchronously if the pipe is not created with FILE_FLAG_OVERLAPPED
132148
do
133149
{
@@ -164,7 +180,7 @@ const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool use
164180
break;
165181

166182
case ERROR_BROKEN_PIPE:
167-
return NULL;
183+
goto exit;
168184

169185
default:
170186
CancelIo(hChildPipeRead);
@@ -175,6 +191,13 @@ const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool use
175191
ffStrbufAppendNS(buffer, nRead, str);
176192
} while (nRead > 0);
177193

194+
exit:
195+
{
196+
DWORD exitCode = 0;
197+
if (GetExitCodeProcess(hProcess, &exitCode) && exitCode != STILL_ACTIVE && exitCode != 0)
198+
return "Child process exited with an error";
199+
}
200+
178201
return NULL;
179202
}
180203

0 commit comments

Comments
 (0)