Skip to content

Commit 35c7070

Browse files
committed
trace/openfiles screen: fix unnecessary copying parent memory page table
posix_spawn() is a more lightweight and modern alternative to fork()/exec(), which are used inside popen(). The main advantage of posix_spawn is that it can create a new process without completely duplicating the address space of the parent process. References: - https://blog.famzah.net/2018/12/19/posix_spawn-performance-benchmarks-and-usage-examples/ - https://lobste.rs/s/smbsd5/fork_road - https://www.reddit.com/r/C_Programming/comments/1lvdhp2/fork_vs_posix_spawn/
1 parent 8211b10 commit 35c7070

2 files changed

Lines changed: 69 additions & 54 deletions

File tree

OpenFilesScreen.c

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ in the source distribution for its full text.
1212
#include <errno.h>
1313
#include <fcntl.h>
1414
#include <inttypes.h>
15+
#include <spawn.h>
1516
#include <stdbool.h>
1617
#include <stdio.h>
1718
#include <stdlib.h>
@@ -27,6 +28,9 @@ in the source distribution for its full text.
2728
#include "XUtils.h"
2829

2930

31+
// Helper to create a mutable string for argv arrays
32+
#define MUTABLE_STR(s) (char[]){s}
33+
3034
// cf. getIndexForType; must be larger than the maximum value returned.
3135
#define LSOF_DATACOL_COUNT 8
3236

@@ -106,32 +110,41 @@ static OpenFiles_ProcessData* OpenFilesScreen_getProcessData(pid_t pid) {
106110
return pdata;
107111
}
108112

109-
pid_t child = fork();
110-
if (child == -1) {
113+
pid_t child;
114+
posix_spawnattr_t attr;
115+
posix_spawnattr_init(&attr);
116+
posix_spawn_file_actions_t fa;
117+
posix_spawn_file_actions_init(&fa);
118+
posix_spawn_file_actions_addclose(&fa, fdpair[0]);
119+
posix_spawn_file_actions_adddup2(&fa, fdpair[1], STDOUT_FILENO);
120+
posix_spawn_file_actions_addclose(&fa, fdpair[1]);
121+
posix_spawn_file_actions_addopen(&fa, STDERR_FILENO, "/dev/null", O_WRONLY | O_NOCTTY, 0);
122+
123+
char buffer[32] = {0};
124+
xSnprintf(buffer, sizeof(buffer), "%d", pid);
125+
126+
char* const args[] = {
127+
MUTABLE_STR("lsof"),
128+
MUTABLE_STR("-P"),
129+
MUTABLE_STR("-o"),
130+
MUTABLE_STR("-p"),
131+
buffer,
132+
MUTABLE_STR("-F"),
133+
NULL
134+
};
135+
136+
int spawn_ret = posix_spawnp(&child, "lsof", &fa, &attr, args, NULL);
137+
138+
posix_spawnattr_destroy(&attr);
139+
posix_spawn_file_actions_destroy(&fa);
140+
141+
if (spawn_ret != 0) {
111142
close(fdpair[1]);
112143
close(fdpair[0]);
113144
pdata->error = 1;
114145
return pdata;
115146
}
116147

117-
if (child == 0) {
118-
close(fdpair[0]);
119-
dup2(fdpair[1], STDOUT_FILENO);
120-
close(fdpair[1]);
121-
int fdnull = open("/dev/null", O_WRONLY);
122-
if (fdnull < 0) {
123-
exit(1);
124-
}
125-
126-
dup2(fdnull, STDERR_FILENO);
127-
close(fdnull);
128-
char buffer[32] = {0};
129-
xSnprintf(buffer, sizeof(buffer), "%d", pid);
130-
// Use of NULL in variadic functions must have a pointer cast.
131-
// The NULL constant is not required by standard to have a pointer type.
132-
execlp("lsof", "lsof", "-P", "-o", "-p", buffer, "-F", (char*)NULL);
133-
exit(127);
134-
}
135148
close(fdpair[1]);
136149

137150
OpenFiles_Data* item = &(pdata->data);

TraceScreen.c

Lines changed: 36 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ in the source distribution for its full text.
1313
#include <errno.h>
1414
#include <fcntl.h>
1515
#include <signal.h>
16+
#include <spawn.h>
1617
#include <stdbool.h>
1718
#include <stdio.h>
1819
#include <stdlib.h>
@@ -28,6 +29,9 @@ in the source distribution for its full text.
2829
#include "XUtils.h"
2930

3031

32+
// Helper to create a mutable string for argv arrays
33+
#define MUTABLE_STR(s) (char[]){s}
34+
3135
static const char* const TraceScreenFunctions[] = {"Search ", "Filter ", "AutoScroll ", "Stop Tracing ", "Done ", NULL};
3236

3337
static const char* const TraceScreenKeys[] = {"F3", "F4", "F8", "F9", "Esc"};
@@ -78,45 +82,43 @@ bool TraceScreen_forkTracer(TraceScreen* this) {
7882
if (fcntl(fdpair[1], F_SETFL, O_NONBLOCK) < 0)
7983
goto err;
8084

81-
pid_t child = fork();
82-
if (child == -1)
85+
pid_t child;
86+
posix_spawnattr_t attr;
87+
posix_spawnattr_init(&attr);
88+
posix_spawn_file_actions_t fa;
89+
posix_spawn_file_actions_init(&fa);
90+
posix_spawn_file_actions_addclose(&fa, fdpair[0]);
91+
posix_spawn_file_actions_adddup2(&fa, fdpair[1], STDOUT_FILENO);
92+
posix_spawn_file_actions_adddup2(&fa, fdpair[1], STDERR_FILENO);
93+
posix_spawn_file_actions_addclose(&fa, fdpair[1]);
94+
95+
char buffer[32] = {0};
96+
xSnprintf(buffer, sizeof(buffer), "%d", Process_getPid(this->super.process));
97+
98+
char* const* args;
99+
#if defined(HTOP_FREEBSD) || defined(HTOP_OPENBSD) || defined(HTOP_NETBSD) || defined(HTOP_DRAGONFLYBSD) || defined(HTOP_SOLARIS)
100+
args = (char* const[]){MUTABLE_STR("truss"), MUTABLE_STR("-s"), MUTABLE_STR("512"), MUTABLE_STR("-p"), buffer, NULL};
101+
#elif defined(HTOP_LINUX)
102+
args = (char* const[]){MUTABLE_STR("strace"), MUTABLE_STR("-T"), MUTABLE_STR("-tt"), MUTABLE_STR("-s"), MUTABLE_STR("512"), MUTABLE_STR("-p"), buffer, NULL};
103+
#else
104+
args = NULL;
105+
#endif
106+
107+
int spawn_ret = args ? posix_spawnp(&child, args[0], &fa, &attr, args, NULL) : -1;
108+
109+
posix_spawnattr_destroy(&attr);
110+
posix_spawn_file_actions_destroy(&fa);
111+
112+
if (spawn_ret != 0) {
83113
goto err;
84-
85-
if (child == 0) {
86-
close(fdpair[0]);
87-
88-
dup2(fdpair[1], STDOUT_FILENO);
89-
dup2(fdpair[1], STDERR_FILENO);
90-
close(fdpair[1]);
91-
92-
char buffer[32] = {0};
93-
xSnprintf(buffer, sizeof(buffer), "%d", Process_getPid(this->super.process));
94-
95-
#if defined(HTOP_FREEBSD) || defined(HTOP_OPENBSD) || defined(HTOP_NETBSD) || defined(HTOP_DRAGONFLYBSD) || defined(HTOP_SOLARIS)
96-
// Use of NULL in variadic functions must have a pointer cast.
97-
// The NULL constant is not required by standard to have a pointer type.
98-
execlp("truss", "truss", "-s", "512", "-p", buffer, (void*)NULL);
99-
100-
// Should never reach here, unless execlp fails ...
101-
const char* message = "Could not execute 'truss'. Please make sure it is available in your $PATH.";
102-
(void)! write(STDERR_FILENO, message, strlen(message));
103-
#elif defined(HTOP_LINUX)
104-
execlp("strace", "strace", "-T", "-tt", "-s", "512", "-p", buffer, (void*)NULL);
105-
106-
// Should never reach here, unless execlp fails ...
107-
const char* message = "Could not execute 'strace'. Please make sure it is available in your $PATH.";
108-
(void)! write(STDERR_FILENO, message, strlen(message));
109-
#else // HTOP_DARWIN, HTOP_PCP == HTOP_UNSUPPORTED
110-
const char* message = "Tracing unavailable on not supported system.";
111-
(void)! write(STDERR_FILENO, message, strlen(message));
112-
#endif
113-
114-
exit(127);
115114
}
116115

117116
FILE* fp = fdopen(fdpair[0], "r");
118-
if (!fp)
117+
if (!fp) {
118+
kill(child, SIGTERM);
119+
waitpid(child, NULL, 0);
119120
goto err;
121+
}
120122

121123
close(fdpair[1]);
122124

0 commit comments

Comments
 (0)