xaizek / vifm (License: GPLv2+) (since 2018-12-07)
Vifm is a file manager with curses interface, which provides Vi[m]-like environment for managing objects within file systems, extended with some useful ideas from mutt.
Clone URLs: https://code.reversed.top/user/xaizek/vifm ssh://rocketgit@code.reversed.top/user/xaizek/vifm
v0.13 v0.13-beta master
Commit 4399e2e2b81c4ef21e05d3461b9192159586d2f5

Fix redirection issues of bg jobs on Windows
Whether created via `:!cmd &` or in Lua.

If the application was closing one of standard streams while using
another one (like `tar` seems to close input stream), this could cause
trouble because streams were the same (Windows doesn't duplicate
configured handles).

This also adds set of proper tests for such cases (all 8 possible
combinations of using/closing streams).
Author: xaizek
Author date (UTC): 2024-03-11 13:58
Committer name: xaizek
Committer date (UTC): 2024-03-11 17:30
Parent(s): 1d3d558f747bd234060940cfb12936b383ba4517
Signing key: 99DC5E4DB05F6BE2
Tree: 01e2847fbeec1d6404a9c317a911230034c9809f
File Lines added Lines deleted
ChangeLog 4 0
src/background.c 89 23
tests/Makefile 2 1
tests/io_tester_app/io_tester.c 90 0
tests/lua/api_jobs.c 93 0
File ChangeLog changed (mode: 100644) (index 7cadacdc2..eb9929482)
211 211 more than a couple of files with identical size and 4096-byte prefix. more than a couple of files with identical size and 4096-byte prefix.
212 212 A regression in 0.13-beta. Thanks to Alexandr Keyp (a.k.a. IAmKapuze). A regression in 0.13-beta. Thanks to Alexandr Keyp (a.k.a. IAmKapuze).
213 213
214 Fixed stream redirection issues with background jobs on Windows (those
215 created via `:!cmd &` or in Lua) if the application was closing one of
216 standard streams.
217
214 218 0.13-beta to 0.13 (2023-04-04) 0.13-beta to 0.13 (2023-04-04)
215 219
216 220 Made "withicase" and "withrcase" affect how files are sorted before Made "withicase" and "withrcase" affect how files are sorted before
File src/background.c changed (mode: 100644) (index 6aa8bd66a..1ce5c1836)
... ... static void report_error_msg(const char title[], const char text[]);
128 128 #endif #endif
129 129 static bg_job_t * launch_external(const char cmd[], BgJobFlags flags, static bg_job_t * launch_external(const char cmd[], BgJobFlags flags,
130 130 ShellRequester by); ShellRequester by);
131 #ifdef _WIN32
132 static int finish_startup_info(STARTUPINFOW *startup);
133 #endif
131 134 static void append_error_msg(bg_job_t *job, const char err_msg[]); static void append_error_msg(bg_job_t *job, const char err_msg[]);
132 135 static void place_on_job_bar(bg_job_t *job); static void place_on_job_bar(bg_job_t *job);
133 136 static void get_off_job_bar(bg_job_t *job); static void get_off_job_bar(bg_job_t *job);
 
... ... launch_external(const char cmd[], BgJobFlags flags, ShellRequester by)
1154 1157
1155 1158 return job; return job;
1156 1159 #else #else
1157 STARTUPINFOW startup = { .dwFlags = STARTF_USESTDHANDLES };
1160 /* Handles are either set below or redirected to NUL in
1161 * finish_startup_info(). */
1162 STARTUPINFOW startup = {
1163 .dwFlags = STARTF_USESTDHANDLES,
1164 .hStdInput = INVALID_HANDLE_VALUE,
1165 .hStdOutput = INVALID_HANDLE_VALUE,
1166 .hStdError = INVALID_HANDLE_VALUE,
1167 };
1158 1168 PROCESS_INFORMATION pinfo; PROCESS_INFORMATION pinfo;
1159 1169 char *sh_cmd; char *sh_cmd;
1160 1170 wchar_t *wide_cmd; wchar_t *wide_cmd;
1161 1171
1162 HANDLE hnul = CreateFileA("NUL", GENERIC_READ | GENERIC_WRITE, 0, NULL,
1163 OPEN_EXISTING, 0, NULL);
1164 if(hnul == INVALID_HANDLE_VALUE)
1165 {
1166 return NULL;
1167 }
1168 startup.hStdInput = hnul;
1169 startup.hStdOutput = hnul;
1170
1171 1172 HANDLE herr = INVALID_HANDLE_VALUE; HANDLE herr = INVALID_HANDLE_VALUE;
1172 1173 if(!merge_streams && !CreatePipe(&herr, &startup.hStdError, NULL, 16*1024)) if(!merge_streams && !CreatePipe(&herr, &startup.hStdError, NULL, 16*1024))
1173 1174 { {
1174 CloseHandle(hnul);
1175 1175 return NULL; return NULL;
1176 1176 } }
1177 1177
 
... ... launch_external(const char cmd[], BgJobFlags flags, ShellRequester by)
1182 1182 { {
1183 1183 CloseHandle(herr); CloseHandle(herr);
1184 1184 } }
1185 CloseHandle(hnul);
1186 1185 return NULL; return NULL;
1187 1186 } }
1188 1187
 
... ... launch_external(const char cmd[], BgJobFlags flags, ShellRequester by)
1196 1195 CloseHandle(herr); CloseHandle(herr);
1197 1196 } }
1198 1197 CloseHandle(hin); CloseHandle(hin);
1199 CloseHandle(hnul);
1200 1198 return NULL; return NULL;
1201 1199 } }
1202 1200
1203 1201 if(merge_streams) if(merge_streams)
1204 1202 { {
1205 startup.hStdError = startup.hStdOutput;
1203 /* Duplicate instead of just assigning so that closing one handle keeps
1204 * the other one operational. */
1205 HANDLE this_process = GetCurrentProcess();
1206 if(!DuplicateHandle(this_process, startup.hStdOutput, this_process,
1207 &startup.hStdError, /*access=*/0, /*inheritable=*/TRUE,
1208 DUPLICATE_SAME_ACCESS))
1209 {
1210 CloseHandle(startup.hStdOutput);
1211 CloseHandle(hout);
1212 CloseHandle(hin);
1213 return NULL;
1214 }
1206 1215 } }
1207 1216 } }
1208 1217
1209 SetHandleInformation(startup.hStdInput, HANDLE_FLAG_INHERIT, 1);
1210 SetHandleInformation(startup.hStdOutput, HANDLE_FLAG_INHERIT, 1);
1211 SetHandleInformation(startup.hStdError, HANDLE_FLAG_INHERIT, 1);
1212
1218 finish_startup_info(&startup);
1213 1219 sh_cmd = win_make_sh_cmd(cmd, by); sh_cmd = win_make_sh_cmd(cmd, by);
1214 1220
1215 1221 wide_cmd = to_wide(sh_cmd); wide_cmd = to_wide(sh_cmd);
1216 1222 int started = CreateProcessW(NULL, wide_cmd, NULL, NULL, 1, CREATE_SUSPENDED, int started = CreateProcessW(NULL, wide_cmd, NULL, NULL, 1, CREATE_SUSPENDED,
1217 1223 NULL, NULL, &startup, &pinfo); NULL, NULL, &startup, &pinfo);
1218 1224 free(wide_cmd); free(wide_cmd);
1219 CloseHandle(hnul);
1225
1220 1226 CloseHandle(startup.hStdInput); CloseHandle(startup.hStdInput);
1221 1227 CloseHandle(startup.hStdOutput); CloseHandle(startup.hStdOutput);
1222 if(startup.hStdError != startup.hStdOutput)
1223 {
1224 CloseHandle(startup.hStdError);
1225 }
1228 CloseHandle(startup.hStdError);
1226 1229
1227 1230 if(!started) if(!started)
1228 1231 { {
 
... ... launch_external(const char cmd[], BgJobFlags flags, ShellRequester by)
1297 1300 #endif #endif
1298 1301 } }
1299 1302
1303 #ifdef _WIN32
1304 /* Makes sure that standard handles of startup structure which weren't
1305 * initialized are redirected to NUL. Returns zero on success. */
1306 static int
1307 finish_startup_info(STARTUPINFOW *startup)
1308 {
1309 int missing = (startup->hStdInput == INVALID_HANDLE_VALUE)
1310 + (startup->hStdOutput == INVALID_HANDLE_VALUE)
1311 + (startup->hStdError == INVALID_HANDLE_VALUE);
1312 if(missing == 0)
1313 {
1314 goto no_redirects;
1315 }
1316
1317 /* Open up to three handles so that child process could close one of them
1318 * while keep using others. */
1319 HANDLE hnul[3];
1320 hnul[0] = CreateFileA("NUL", GENERIC_READ | GENERIC_WRITE,
1321 FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING,
1322 FILE_ATTRIBUTE_NORMAL, NULL);
1323 if(hnul[0] == INVALID_HANDLE_VALUE)
1324 {
1325 return 1;
1326 }
1327
1328 int i;
1329 HANDLE this_process = GetCurrentProcess();
1330 for(i = 1; i < missing; ++i)
1331 {
1332 if(!DuplicateHandle(this_process, hnul[0], this_process, &hnul[i],
1333 /*access=*/0, /*inheritable=*/TRUE, DUPLICATE_SAME_ACCESS))
1334 {
1335 int j;
1336 for(j = 0; j < i; ++j)
1337 {
1338 CloseHandle(hnul[j]);
1339 }
1340 return 1;
1341 }
1342 }
1343
1344 if(startup->hStdInput == INVALID_HANDLE_VALUE)
1345 {
1346 startup->hStdInput = hnul[--missing];
1347 }
1348 if(startup->hStdOutput == INVALID_HANDLE_VALUE)
1349 {
1350 startup->hStdOutput = hnul[--missing];
1351 }
1352 if(startup->hStdError == INVALID_HANDLE_VALUE)
1353 {
1354 startup->hStdError = hnul[--missing];
1355 }
1356
1357 no_redirects:
1358 SetHandleInformation(startup->hStdInput, HANDLE_FLAG_INHERIT, 1);
1359 SetHandleInformation(startup->hStdOutput, HANDLE_FLAG_INHERIT, 1);
1360 SetHandleInformation(startup->hStdError, HANDLE_FLAG_INHERIT, 1);
1361
1362 return 0;
1363 }
1364 #endif
1365
1300 1366 int int
1301 1367 bg_execute(const char descr[], const char op_descr[], int total, int important, bg_execute(const char descr[], const char op_descr[], int total, int important,
1302 1368 bg_task_func task_func, void *args) bg_task_func task_func, void *args)
File tests/Makefile changed (mode: 100644) (index 8d429dbd6..18d003419)
... ... suites += colmgr column_view viewcolumns_parser
98 98 suites += bmarks escape fileops filetype filter lua menus misc undo utils suites += bmarks escape fileops filetype filter lua menus misc undo utils
99 99
100 100 # these are built, but not automatically executed # these are built, but not automatically executed
101 apps := fuzz regs_shmem_app
101 apps := fuzz io_tester_app regs_shmem_app
102 102
103 103 # obtain list of sources that are being tested # obtain list of sources that are being tested
104 104 vifm_src := ./ cfg/ compat/ engine/ int/ io/ io/private/ lua/ lua/lua/ menus/ vifm_src := ./ cfg/ compat/ engine/ int/ io/ io/private/ lua/ lua/lua/ menus/
 
... ... $(foreach suite, $(suites) $(apps), $(eval $(call suite_template,$(suite))))
377 377
378 378 # extra dependencies between test fixtures # extra dependencies between test fixtures
379 379 misc: | $(regs_shmem_app.bin) misc: | $(regs_shmem_app.bin)
380 lua: | $(io_tester_app.bin)
380 381
381 382 # import dependencies calculated by the compiler # import dependencies calculated by the compiler
382 383 include $(wildcard $(deps) \ include $(wildcard $(deps) \
File tests/io_tester_app/io_tester.c added (mode: 100644) (index 000000000..35144668e)
1 #include <stdio.h> /* EOF FILE fclose() fflush() fgets() */
2 #include <string.h> /* strcmp() strlen() strspn() */
3
4 /*
5 * Usage: io_tester modes
6 *
7 * `modes` must be a three-letter sequence of:
8 * * c -- close
9 * * k -- keep
10 * * u -- use
11 *
12 * A letter corresponds to stdin, stdout and stderr in this order.
13 *
14 * Example modes: kcc, cuc, ccu
15 *
16 * When a file descriptor is being used, stdin expects "stdin" string and
17 * stdout and stderr print "stdout" and "stderr" respectively.
18 */
19
20 int
21 main(int argc, char *argv[])
22 {
23 if(argc != 2)
24 {
25 return 10;
26 }
27
28 const char *modes = argv[1];
29 if(strlen(modes) != 3)
30 {
31 return 11;
32 }
33 if(strspn(modes, "cku") != 3)
34 {
35 return 12;
36 }
37
38 FILE *fps[3] = { stdin, stdout, stderr };
39 const char *data[3] = { "stdin", "stdout", "stderr" };
40 int i;
41
42 /* Start by closing descriptors to make sure others can still be used. */
43 for(i = 0; i < 3; ++i)
44 {
45 if(modes[i] == 'c')
46 {
47 if(fclose(fps[i]) != 0)
48 {
49 return 20 + i;
50 }
51 }
52 }
53
54 for(i = 0; i < 3; ++i)
55 {
56 if(modes[i] != 'u')
57 {
58 continue;
59 }
60
61 if(i == 0)
62 {
63 char buf[16];
64 if(fgets(buf, sizeof(buf), fps[i]) != buf)
65 {
66 return 30;
67 }
68 if(strcmp(buf, data[i]) != 0)
69 {
70 return 31;
71 }
72 }
73 else
74 {
75 if(fputs(data[i], fps[i]) == EOF)
76 {
77 return 40 + i;
78 }
79 if(fflush(fps[i]) == EOF)
80 {
81 return 50 + i;
82 }
83 }
84 }
85
86 return 0;
87 }
88
89 /* vim: set tabstop=2 softtabstop=2 shiftwidth=2 noexpandtab cinoptions-=(0 : */
90 /* vim: set cinoptions+=t0 filetype=c : */
File tests/lua/api_jobs.c changed (mode: 100644) (index 2d53df28c..e393282a8)
2 2
3 3 #include <unistd.h> /* usleep() */ #include <unistd.h> /* usleep() */
4 4
5 #include <stdlib.h> /* getenv() */
6
5 7 #include "../../src/engine/var.h" #include "../../src/engine/var.h"
6 8 #include "../../src/engine/variables.h" #include "../../src/engine/variables.h"
7 9 #include "../../src/lua/vlua.h" #include "../../src/lua/vlua.h"
 
12 14
13 15 #include "asserts.h" #include "asserts.h"
14 16
17 static void setup_io_tester(void);
15 18 static void wait_for_job(void); static void wait_for_job(void);
16 19
17 20 static vlua_t *vlua; static vlua_t *vlua;
 
... ... SETUP()
25 28 TEARDOWN() TEARDOWN()
26 29 { {
27 30 vlua_finish(vlua); vlua_finish(vlua);
31 /* Without waiting for jobs to finish tests hang in Wine but not on Windows
32 * which makes it hard to debug. Unclear whether this indicates a bug in Vifm
33 * or in Wine. */
34 wait_for_all_bg();
28 35 conf_teardown(); conf_teardown();
29 36 } }
30 37
 
... ... TEST(vifmjob_no_in)
146 153 "print(job:stdin() and 'FAIL')"); "print(job:stdin() and 'FAIL')");
147 154 } }
148 155
156 TEST(vifmjob_io_close_none)
157 {
158 setup_io_tester();
159 GLUA_EQ(vlua, "",
160 "job = vifm.startjob { cmd = io_tester..' uuu', iomode = 'w' }");
161 GLUA_EQ(vlua, "true", "print(job:stdin():write('stdin') == job:stdin())");
162 GLUA_EQ(vlua, "0", "print(job:exitcode())");
163 GLUA_EQ(vlua, "stderr", "print(job:errors())");
164 }
165
166 TEST(vifmjob_io_close_all)
167 {
168 setup_io_tester();
169 GLUA_EQ(vlua, "0",
170 "job = vifm.startjob { cmd = io_tester..' ccc', iomode = '' }"
171 "print(job:exitcode())"); // got 21
172 }
173
174 TEST(vifmjob_io_close_in)
175 {
176 setup_io_tester();
177 GLUA_EQ(vlua, "", "job = vifm.startjob { cmd = io_tester..' cuu' }");
178 GLUA_EQ(vlua, "stdout", "print(job:stdout():lines()())");
179 GLUA_EQ(vlua, "0", "print(job:exitcode())");
180 GLUA_EQ(vlua, "stderr", "print(job:errors())");
181 }
182
183 TEST(vifmjob_io_close_out)
184 {
185 setup_io_tester();
186 GLUA_EQ(vlua, "",
187 "job = vifm.startjob { cmd = io_tester..' ucu', iomode = 'w' }");
188 GLUA_EQ(vlua, "true", "print(job:stdin():write('stdin') == job:stdin())");
189 GLUA_EQ(vlua, "0", "print(job:exitcode())");
190 GLUA_EQ(vlua, "stderr", "print(job:errors())");
191 }
192
193 TEST(vifmjob_io_close_err)
194 {
195 setup_io_tester();
196 GLUA_EQ(vlua, "",
197 "job = vifm.startjob { cmd = io_tester..' uuc', iomode = 'w' }");
198 GLUA_EQ(vlua, "true", "print(job:stdin():write('stdin') == job:stdin())");
199 GLUA_EQ(vlua, "0", "print(job:exitcode())");
200 GLUA_EQ(vlua, "", "print(job:errors())");
201 }
202
203 TEST(vifmjob_io_keep_in)
204 {
205 setup_io_tester();
206 GLUA_EQ(vlua, "",
207 "job = vifm.startjob { cmd = io_tester..' ucc', iomode = 'w' }");
208 GLUA_EQ(vlua, "true", "print(job:stdin():write('stdin') == job:stdin())");
209 GLUA_EQ(vlua, "0", "print(job:exitcode())");
210 GLUA_EQ(vlua, "", "print(job:errors())");
211 }
212
213 TEST(vifmjob_io_keep_out)
214 {
215 setup_io_tester();
216 GLUA_EQ(vlua, "", "job = vifm.startjob { cmd = io_tester..' cuc' }");
217 GLUA_EQ(vlua, "stdout", "print(job:stdout():lines()())");
218 GLUA_EQ(vlua, "0", "print(job:exitcode())");
219 GLUA_EQ(vlua, "", "print(job:errors())");
220 }
221
222 TEST(vifmjob_io_keep_err)
223 {
224 setup_io_tester();
225 GLUA_EQ(vlua, "", "job = vifm.startjob { cmd = io_tester..' ccu' }");
226 GLUA_EQ(vlua, "0", "print(job:exitcode())");
227 GLUA_EQ(vlua, "stderr", "print(job:errors())");
228 }
229
149 230 TEST(vifmjob_onexit_good) TEST(vifmjob_onexit_good)
150 231 { {
151 232 var_t var = var_from_int(0); var_t var = var_from_int(0);
 
... ... TEST(vifmjob_onexit_bad)
181 262 ui_sb_last()); ui_sb_last());
182 263 } }
183 264
265 static void
266 setup_io_tester(void)
267 {
268 const int debug = (getenv("DEBUG") != NULL);
269 const char *bin_path = (debug ? "bin/debug" : "bin");
270
271 char set_cmd_lua[64];
272 snprintf(set_cmd_lua, sizeof(set_cmd_lua),
273 "io_tester = '%s" SL "io_tester_app'", bin_path);
274 assert_success(vlua_run_string(vlua, set_cmd_lua));
275 }
276
184 277 static void static void
185 278 wait_for_job(void) wait_for_job(void)
186 279 { {
Hints
Before first commit, do not forget to setup your git environment:
git config --global user.name "your_name_here"
git config --global user.email "your@email_here"

Clone this repository using HTTP(S):
git clone https://code.reversed.top/user/xaizek/vifm

Clone this repository using ssh (do not forget to upload a key first):
git clone ssh://rocketgit@code.reversed.top/user/xaizek/vifm

You are allowed to anonymously push to this repository.
This means that your pushed commits will automatically be transformed into a pull request:
... clone the repository ...
... make some changes and some commits ...
git push origin master