@cryptotaxi247 / netdata-1 / commits / 47afd05e2

spawn server fixes 3 (#18117)

always send back the spawn server response to the socket

Costa Tsaousis committed Jul 11, 2024 at 21:02 UTC 47afd05e246830982f1b0263c818392b63086462

2 files changed +107 -98

src/libnetdata/spawn_server/spawn_server.c

+96 -88

@@ -355,7 +355,7 @@ static void spawn_server_send_status_ping(int fd) {

355 nd_log(NDLS_COLLECTORS, NDLP_ERR, "SPAWN SERVER: Cannot send ping status report");

356 }

357

358 -static void spawn_server_send_status_success(int fd) {

358 +static void spawn_server_send_status_success(SPAWN_REQUEST *rq) {

359 const struct status_report sr = {

360 .status = STATUS_REPORT_STARTED,

361 .started = {

@@ -363,11 +363,11 @@ static void spawn_server_send_status_success(int fd) {

363 },

364 };

365

366 - if(write(fd, &sr, sizeof(sr)) != sizeof(sr))

366 + if(write(rq->sock, &sr, sizeof(sr)) != sizeof(sr))

367 nd_log(NDLS_COLLECTORS, NDLP_ERR, "SPAWN SERVER: Cannot send success status report");

368 }

369

370 -static void spawn_server_send_status_failure(int fd) {

370 +static void spawn_server_send_status_failure(SPAWN_REQUEST *rq) {

371 struct status_report sr = {

372 .status = STATUS_REPORT_FAILED,

373 .failed = {

@@ -375,11 +375,11 @@ static void spawn_server_send_status_failure(int fd) {

375 },

376 };

377

378 - if(write(fd, &sr, sizeof(sr)) != sizeof(sr))

378 + if(write(rq->sock, &sr, sizeof(sr)) != sizeof(sr))

379 nd_log(NDLS_COLLECTORS, NDLP_ERR, "SPAWN SERVER: Cannot send failure status report");

380 }

381

382 -static void spawn_server_send_status_exit(int fd, int waitpid_status) {

382 +static void spawn_server_send_status_exit(SPAWN_REQUEST *rq, int waitpid_status) {

383 struct status_report sr = {

384 .status = STATUS_REPORT_EXITED,

385 .exited = {

@@ -387,11 +387,11 @@ static void spawn_server_send_status_exit(int fd, int waitpid_status) {

387 },

388 };

389

390 - if(write(fd, &sr, sizeof(sr)) != sizeof(sr))

390 + if(write(rq->sock, &sr, sizeof(sr)) != sizeof(sr))

391 nd_log(NDLS_COLLECTORS, NDLP_ERR, "SPAWN SERVER: Cannot send exit status report");

392 }

393

394 -static void spawn_server_run_child(SPAWN_SERVER *server, SPAWN_REQUEST *request) {

394 +static void spawn_server_run_child(SPAWN_SERVER *server, SPAWN_REQUEST *rq) {

395 // fprintf(stderr, "CHILD: running request %zu on pid %d\n", request->request_id, getpid());

396

397 // close the server sockets;

@@ -402,67 +402,67 @@ static void spawn_server_run_child(SPAWN_SERVER *server, SPAWN_REQUEST *request)

402 // set the process name

403 {

404 char buf[15];

405 - snprintfz(buf, sizeof(buf), "chld-%zu-r%zu", server->id, request->request_id);

405 + snprintfz(buf, sizeof(buf), "chld-%zu-r%zu", server->id, rq->request_id);

406 os_setproctitle(buf, server->argc, server->argv);

407 }

408

409 // get the fds from the request

410 - int stdin_fd = request->fds[0];

411 - int stdout_fd = request->fds[1];

412 - int stderr_fd = request->fds[2];

413 - int custom_fd = request->fds[3];

410 + int stdin_fd = rq->fds[0];

411 + int stdout_fd = rq->fds[1];

412 + int stderr_fd = rq->fds[2];

413 + int custom_fd = rq->fds[3];

414

415 // change stdio fds to the ones in the request

416 if (dup2(stdin_fd, STDIN_FILENO) == -1) {

417 - spawn_server_send_status_failure(stdout_fd);

417 + spawn_server_send_status_failure(rq);

418 exit(1);

419 }

420 if (dup2(stdout_fd, STDOUT_FILENO) == -1) {

421 - spawn_server_send_status_failure(stdout_fd);

421 + spawn_server_send_status_failure(rq);

422 exit(1);

423 }

424 if (dup2(stderr_fd, STDERR_FILENO) == -1) {

425 - spawn_server_send_status_failure(stdout_fd);

425 + spawn_server_send_status_failure(rq);

426 exit(1);

427 }

428

429 // close the excess fds

430 - close(stdin_fd); stdin_fd = request->fds[0] = STDIN_FILENO;

431 - close(stdout_fd); stdout_fd = request->fds[1] = STDOUT_FILENO;

432 - close(stderr_fd); stderr_fd = request->fds[2] = STDERR_FILENO;

430 + close(stdin_fd); stdin_fd = rq->fds[0] = STDIN_FILENO;

431 + close(stdout_fd); stdout_fd = rq->fds[1] = STDOUT_FILENO;

432 + close(stderr_fd); stderr_fd = rq->fds[2] = STDERR_FILENO;

433

434 // overwrite the process environment

435 - environ = (char **)request->environment;

435 + environ = (char **)rq->environment;

436

437 // Perform different actions based on the type

438 - switch (request->type) {

438 + switch (rq->type) {

439

440 case SPAWN_INSTANCE_TYPE_EXEC:

441 - spawn_server_send_status_success(request->socket);

442 - close(request->socket); request->socket = -1;

441 + spawn_server_send_status_success(rq);

442 + close(rq->sock); rq->sock = -1;

443 close(custom_fd); custom_fd = -1;

444 - execvp(request->argv[0], (char **)request->argv);

444 + execvp(rq->argv[0], (char **)rq->argv);

445 nd_log(NDLS_COLLECTORS, NDLP_ERR,

446 "SPAWN SERVER: Failed to execute command of request No %zu (argv[0] = '%s')",

447 - request->request_id, request->argv[0]);

447 + rq->request_id, rq->argv[0]);

448 exit(1);

449 break;

450

451 case SPAWN_INSTANCE_TYPE_CALLBACK:

452 if(server->cb == NULL) {

453 errno = ENOENT;

454 - spawn_server_send_status_failure(request->socket);

455 - close(request->socket); request->socket = -1;

454 + spawn_server_send_status_failure(rq);

455 + close(rq->sock); rq->sock = -1;

456 exit(1);

457 }

458 - spawn_server_send_status_success(request->socket);

459 - close(request->socket); request->socket = -1;

460 - server->cb(request);

458 + spawn_server_send_status_success(rq);

459 + close(rq->sock); rq->sock = -1;

460 + server->cb(rq);

461 exit(0);

462 break;

463

464 default:

465 - nd_log(NDLS_COLLECTORS, NDLP_ERR, "SPAWN SERVER: unknown request type %u", request->type);

465 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "SPAWN SERVER: unknown request type %u", rq->type);

466 exit(1);

467 }

468 }

@@ -714,10 +714,10 @@ static bool spawn_server_send_request(ND_UUID *magic, SPAWN_REQUEST *request) {

714

715 memcpy(CMSG_DATA(cmsg), request->fds, sizeof(int) * SPAWN_SERVER_TRANSFER_FDS);

716

717 - int rc = sendmsg(request->socket, &msg, 0);

717 + int rc = sendmsg(request->sock, &msg, 0);

718

719 if (rc < 0) {

720 - nd_log(NDLS_COLLECTORS, NDLP_ERR, "SPAWN PARENT: Failed to sendmsg() request to spawn server using socket %d.", request->socket);

720 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "SPAWN PARENT: Failed to sendmsg() request to spawn server using socket %d.", request->sock);

721 goto cleanup;

722 }

723 else {

@@ -737,7 +737,7 @@ static void request_free(SPAWN_REQUEST *rq) {

737 if(rq->fds[1] != -1) close(rq->fds[1]);

738 if(rq->fds[2] != -1) close(rq->fds[2]);

739 if(rq->fds[3] != -1) close(rq->fds[3]);

740 - if(rq->socket != -1) close(rq->socket);

740 + if(rq->sock != -1) close(rq->sock);

741 freez((void *)rq->argv);

742 freez((void *)rq->environment);

743 freez((void *)rq->data);

@@ -745,6 +745,62 @@ static void request_free(SPAWN_REQUEST *rq) {

745 freez((void *)rq);

746 }

747

748 +static void spawn_server_execute_request(SPAWN_SERVER *server, SPAWN_REQUEST *rq) {

749 + switch(rq->type) {

750 + case SPAWN_INSTANCE_TYPE_EXEC:

751 + if(rq->argv) {

752 + CLEAN_BUFFER *wb = argv_to_cmdline_buffer(rq->argv);

753 + rq->cmdline = strdupz(buffer_tostring(wb));

754 + }

755 + break;

756 +

757 + case SPAWN_INSTANCE_TYPE_CALLBACK:

758 + rq->cmdline = strdupz("callback() function");

759 + break;

760 +

761 + default:

762 + rq->cmdline = strdupz("[unknown request type]");

763 + break;

764 + }

765 +

766 + pid_t pid = fork();

767 + if (pid < 0) {

768 + // fork failed

769 +

770 + nd_log(NDLS_COLLECTORS, NDLP_ERR, "SPAWN SERVER: Failed to fork() child.");

771 + spawn_server_send_status_failure(rq);

772 + request_free(rq);

773 + return;

774 + }

775 + else if (pid == 0) {

776 + // the child

777 +

778 + spawn_server_run_child(server, rq);

779 + exit(63);

780 + }

781 +

782 + // the parent

783 + rq->pid = pid;

784 +

785 + // do not keep data we don't need at the parent

786 + freez((void *)rq->environment); rq->environment = NULL;

787 + freez((void *)rq->argv); rq->argv = NULL;

788 + freez((void *)rq->data); rq->data = NULL;

789 + rq->data_size = 0;

790 +

791 + // do not keep fds we don't need at the parent

792 + if(rq->fds[0] != -1) { close(rq->fds[0]); rq->fds[0] = -1; }

793 + if(rq->fds[1] != -1) { close(rq->fds[1]); rq->fds[1] = -1; }

794 + if(rq->fds[2] != -1) { close(rq->fds[2]); rq->fds[2] = -1; }

795 + if(rq->fds[3] != -1) { close(rq->fds[3]); rq->fds[3] = -1; }

796 +

797 + // keep it in the list

798 + DOUBLE_LINKED_LIST_APPEND_ITEM_UNSAFE(spawn_server_requests, rq, prev, next);

799 +

800 + // do not fork this socket on other children

801 + sock_setcloexec(rq->sock);

802 +}

803 +

804 static void spawn_server_receive_request(int sock, SPAWN_SERVER *server) {

805 struct msghdr msg = {0};

806 struct iovec iov[7];

@@ -881,7 +937,7 @@ static void spawn_server_receive_request(int sock, SPAWN_SERVER *server) {

937 *rq = (SPAWN_REQUEST){

938 .pid = 0,

939 .request_id = request_id,

884 - .socket = sock,

940 + .sock = sock,

941 .fds = {

942 [0] = stdin_fd,

943 [1] = stdout_fd,

@@ -895,59 +951,11 @@ static void spawn_server_receive_request(int sock, SPAWN_SERVER *server) {

951 .type = type

952 };

953

898 - switch(rq->type) {

899 - case SPAWN_INSTANCE_TYPE_EXEC:

900 - if(rq->argv) {

901 - CLEAN_BUFFER *wb = argv_to_cmdline_buffer(rq->argv);

902 - rq->cmdline = strdupz(buffer_tostring(wb));

903 - }

904 - break;

905 -

906 - case SPAWN_INSTANCE_TYPE_CALLBACK:

907 - rq->cmdline = strdupz("callback() function");

908 - break;

909 -

910 - default:

911 - rq->cmdline = strdupz("[unknown request type]");

912 - break;

913 - }

914 -

915 - pid_t pid = fork();

916 - if (pid == 0) {

917 - // the child

918 - spawn_server_run_child(server, rq);

919 - exit(63);

920 - }

921 - else if (pid > 0) {

922 - // the parent

923 - rq->pid = pid;

924 -

925 - // do not keep data we don't need at the parent

926 - freez(envp_encoded); freez((void *)rq->environment); rq->environment = NULL;

927 - freez(argv_encoded); freez((void *)rq->argv); rq->argv = NULL;

928 - freez((void *)rq->data); rq->data = NULL;

929 - rq->data_size = 0;

930 -

931 - // do not keep fds we don't need at the parent

932 - if(rq->fds[0] != -1) { close(rq->fds[0]); rq->fds[0] = -1; }

933 - if(rq->fds[1] != -1) { close(rq->fds[1]); rq->fds[1] = -1; }

934 - if(rq->fds[2] != -1) { close(rq->fds[2]); rq->fds[2] = -1; }

935 - if(rq->fds[3] != -1) { close(rq->fds[3]); rq->fds[3] = -1; }

936 -

937 - // keep it in the list

938 - DOUBLE_LINKED_LIST_APPEND_ITEM_UNSAFE(spawn_server_requests, rq, prev, next);

939 -

940 - // do not fork this socket on other children

941 - sock_setcloexec(rq->socket);

942 -

943 - // done with this

944 - return;

945 - }

954 + // all allocations given to the request are now handled by this

955 + spawn_server_execute_request(server, rq);

956

947 - nd_log(NDLS_COLLECTORS, NDLP_ERR, "SPAWN SERVER: Failed to fork() child.");

948 - spawn_server_send_status_failure(stdout_fd);

949 - // the other allocations (envp, argv, data) will be free'd at cleanup

950 - request_free(rq);

957 + // since we make rq->argv and rq->environment NULL when we keep it,

958 + // we don't need these anymore.

959 freez(envp_encoded);

960 freez(argv_encoded);

961 return;

@@ -1037,7 +1045,7 @@ static void spawn_server_process_sigchld(void) {

1045 }

1046

1047 if(send_report_remove_request && rq) {

1040 - spawn_server_send_status_exit(rq->socket, status);

1048 + spawn_server_send_status_exit(rq, status);

1049 DOUBLE_LINKED_LIST_REMOVE_ITEM_UNSAFE(spawn_server_requests, rq, prev, next);

1050 request_free(rq);

1051 }

@@ -1398,7 +1406,7 @@ SPAWN_INSTANCE* spawn_server_exec(SPAWN_SERVER *server, int stderr_fd, int custo

1406

1407 SPAWN_REQUEST request = {

1408 .request_id = __atomic_add_fetch(&server->request_id, 1, __ATOMIC_RELAXED),

1401 - .socket = instance->client_sock,

1409 + .sock = instance->client_sock,

1410 .fds = {

1411 [0] = pipe_stdin[0],

1412 [1] = pipe_stdout[1],

src/libnetdata/spawn_server/spawn_server.h

+11 -10

@@ -22,17 +22,18 @@ typedef enum __attribute__((packed)) {

22 // this is only used publicly for SPAWN_INSTANCE_TYPE_CALLBACK

23 // which is not available in Windows

24 typedef struct spawn_request {

25 - const char *cmdline;

26 - size_t request_id;

27 - pid_t pid;

28 - int socket;

25 + const char *cmdline; // the cmd line of the command we should run

26 + size_t request_id; // the incremental request id

27 + pid_t pid; // the pid of the child

28 + int sock; // the socket for this request

29 int fds[SPAWN_SERVER_TRANSFER_FDS]; // 0 = stdin, 1 = stdout, 2 = stderr, 3 = custom

30 - const char **environment;

31 - const char **argv;

32 - const void *data;

33 - size_t data_size;

34 - SPAWN_INSTANCE_TYPE type;

35 - struct spawn_request *prev, *next;

30 + const char **environment; // the environment of the parent process

31 + const char **argv; // the command line and its parameters

32 + const void *data; // the data structure for the callback

33 + size_t data_size; // the data structure size

34 + SPAWN_INSTANCE_TYPE type; // the type of the request

35 +

36 + struct spawn_request *prev, *next; // linking of active requests at the spawn server

37 } SPAWN_REQUEST;

39 typedef void (*spawn_request_callback_t)(SPAWN_REQUEST *request);