t/helper/simple-ipc: convert test-simple-ipc to use start_bg_command
Convert test helper to use `start_bg_command()` when spawning a server daemon in the background rather than blocks of platform-specific code. Also, while here, remove _() translation around error messages since this is a test helper and not Git code. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff Hostetler committed
Sep 20, 2021 at 15:36 UTC
05881a6fc9020b5c227a98490bf256d129da01f6
1 file changed
+43
-156
t/helper/test-simple-ipc.c
+43
-156
@@ -9,6 +9,7 @@
9
#include "parse-options.h"
10
#include "thread-utils.h"
11
#include "strvec.h"
12
+#include "run-command.h"
13
14
#ifndef SUPPORTS_SIMPLE_IPC
15
int cmd__simple_ipc(int argc, const char **argv)
@@ -267,185 +268,71 @@ static int daemon__run_server(void)
268
*/
269
ret = ipc_server_run(cl_args.path, &opts, test_app_cb, (void*)&my_app_data);
270
if (ret == -2)
270
- error(_("socket/pipe already in use: '%s'"), cl_args.path);
271
+ error("socket/pipe already in use: '%s'", cl_args.path);
272
else if (ret == -1)
272
- error_errno(_("could not start server on: '%s'"), cl_args.path);
273
+ error_errno("could not start server on: '%s'", cl_args.path);
274
275
return ret;
276
}
277
277
-#ifndef GIT_WINDOWS_NATIVE
278
-/*
279
- * This is adapted from `daemonize()`. Use `fork()` to directly create and
280
- * run the daemon in a child process.
281
- */
282
-static int spawn_server(pid_t *pid)
283
-{
284
- struct ipc_server_opts opts = {
285
- .nr_threads = cl_args.nr_threads,
286
- };
278
+static start_bg_wait_cb bg_wait_cb;
279
288
- *pid = fork();
289
-
290
- switch (*pid) {
291
- case 0:
292
- if (setsid() == -1)
293
- error_errno(_("setsid failed"));
294
- close(0);
295
- close(1);
296
- close(2);
297
- sanitize_stdfds();
280
+static int bg_wait_cb(const struct child_process *cp, void *cb_data)
281
+{
282
+ int s = ipc_get_active_state(cl_args.path);
283
299
- return ipc_server_run(cl_args.path, &opts, test_app_cb,
300
- (void*)&my_app_data);
284
+ switch (s) {
285
+ case IPC_STATE__LISTENING:
286
+ /* child is "ready" */
287
+ return 0;
288
302
- case -1:
303
- return error_errno(_("could not spawn daemon in the background"));
289
+ case IPC_STATE__NOT_LISTENING:
290
+ case IPC_STATE__PATH_NOT_FOUND:
291
+ /* give child more time */
292
+ return 1;
293
294
default:
306
- return 0;
295
+ case IPC_STATE__INVALID_PATH:
296
+ case IPC_STATE__OTHER_ERROR:
297
+ /* all the time in world won't help */
298
+ return -1;
299
}
300
}
309
-#else
310
-/*
311
- * Conceptually like `daemonize()` but different because Windows does not
312
- * have `fork(2)`. Spawn a normal Windows child process but without the
313
- * limitations of `start_command()` and `finish_command()`.
314
- */
315
-static int spawn_server(pid_t *pid)
316
-{
317
- char test_tool_exe[MAX_PATH];
318
- struct strvec args = STRVEC_INIT;
319
- int in, out;
320
-
321
- GetModuleFileNameA(NULL, test_tool_exe, MAX_PATH);
322
-
323
- in = open("/dev/null", O_RDONLY);
324
- out = open("/dev/null", O_WRONLY);
325
-
326
- strvec_push(&args, test_tool_exe);
327
- strvec_push(&args, "simple-ipc");
328
- strvec_push(&args, "run-daemon");
329
- strvec_pushf(&args, "--name=%s", cl_args.path);
330
- strvec_pushf(&args, "--threads=%d", cl_args.nr_threads);
331
-
332
- *pid = mingw_spawnvpe(args.v[0], args.v, NULL, NULL, in, out, out);
333
- close(in);
334
- close(out);
335
-
336
- strvec_clear(&args);
301
338
- if (*pid < 0)
339
- return error(_("could not spawn daemon in the background"));
340
-
341
- return 0;
342
-}
343
-#endif
344
-
345
-/*
346
- * This is adapted from `wait_or_whine()`. Watch the child process and
347
- * let it get started and begin listening for requests on the socket
348
- * before reporting our success.
349
- */
350
-static int wait_for_server_startup(pid_t pid_child)
302
+static int daemon__start_server(void)
303
{
352
- int status;
353
- pid_t pid_seen;
354
- enum ipc_active_state s;
355
- time_t time_limit, now;
304
+ struct child_process cp = CHILD_PROCESS_INIT;
305
+ enum start_bg_result sbgr;
306
357
- time(&time_limit);
358
- time_limit += cl_args.max_wait_sec;
307
+ strvec_push(&cp.args, "test-tool");
308
+ strvec_push(&cp.args, "simple-ipc");
309
+ strvec_push(&cp.args, "run-daemon");
310
+ strvec_pushf(&cp.args, "--name=%s", cl_args.path);
311
+ strvec_pushf(&cp.args, "--threads=%d", cl_args.nr_threads);
312
360
- for (;;) {
361
- pid_seen = waitpid(pid_child, &status, WNOHANG);
313
+ cp.no_stdin = 1;
314
+ cp.no_stdout = 1;
315
+ cp.no_stderr = 1;
316
363
- if (pid_seen == -1)
364
- return error_errno(_("waitpid failed"));
317
+ sbgr = start_bg_command(&cp, bg_wait_cb, NULL, cl_args.max_wait_sec);
318
366
- else if (pid_seen == 0) {
367
- /*
368
- * The child is still running (this should be
369
- * the normal case). Try to connect to it on
370
- * the socket and see if it is ready for
371
- * business.
372
- *
373
- * If there is another daemon already running,
374
- * our child will fail to start (possibly
375
- * after a timeout on the lock), but we don't
376
- * care (who responds) if the socket is live.
377
- */
378
- s = ipc_get_active_state(cl_args.path);
379
- if (s == IPC_STATE__LISTENING)
380
- return 0;
381
-
382
- time(&now);
383
- if (now > time_limit)
384
- return error(_("daemon not online yet"));
385
-
386
- continue;
387
- }
319
+ switch (sbgr) {
320
+ case SBGR_READY:
321
+ return 0;
322
389
- else if (pid_seen == pid_child) {
390
- /*
391
- * The new child daemon process shutdown while
392
- * it was starting up, so it is not listening
393
- * on the socket.
394
- *
395
- * Try to ping the socket in the odd chance
396
- * that another daemon started (or was already
397
- * running) while our child was starting.
398
- *
399
- * Again, we don't care who services the socket.
400
- */
401
- s = ipc_get_active_state(cl_args.path);
402
- if (s == IPC_STATE__LISTENING)
403
- return 0;
323
+ default:
324
+ case SBGR_ERROR:
325
+ case SBGR_CB_ERROR:
326
+ return error("daemon failed to start");
327
405
- /*
406
- * We don't care about the WEXITSTATUS() nor
407
- * any of the WIF*(status) values because
408
- * `cmd__simple_ipc()` does the `!!result`
409
- * trick on all function return values.
410
- *
411
- * So it is sufficient to just report the
412
- * early shutdown as an error.
413
- */
414
- return error(_("daemon failed to start"));
415
- }
328
+ case SBGR_TIMEOUT:
329
+ return error("daemon not online yet");
330
417
- else
418
- return error(_("waitpid is confused"));
331
+ case SBGR_DIED:
332
+ return error("daemon terminated");
333
}
334
}
335
422
-/*
423
- * This process will start a simple-ipc server in a background process and
424
- * wait for it to become ready. This is like `daemonize()` but gives us
425
- * more control and better error reporting (and makes it easier to write
426
- * unit tests).
427
- */
428
-static int daemon__start_server(void)
429
-{
430
- pid_t pid_child;
431
- int ret;
432
-
433
- /*
434
- * Run the actual daemon in a background process.
435
- */
436
- ret = spawn_server(&pid_child);
437
- if (pid_child <= 0)
438
- return ret;
439
-
440
- /*
441
- * Let the parent wait for the child process to get started
442
- * and begin listening for requests on the socket.
443
- */
444
- ret = wait_for_server_startup(pid_child);
445
-
446
- return ret;
447
-}
448
-
336
/*
337
* This process will run a quick probe to see if a simple-ipc server
338
* is active on this path.
@@ -548,7 +435,7 @@ static int client__stop_server(void)
435
436
time(&now);
437
if (now > time_limit)
551
- return error(_("daemon has not shutdown yet"));
438
+ return error("daemon has not shutdown yet");
439
}
440
}
441