Skip to content

Commit 94b570b

Browse files
committed
cmd: keep connection cycle safe on pre-5.3 kernels
Boost.Process v2 uses pidfd_open on Linux, which requires kernel 5.3+. On Ubuntu 18.04 (kernel 4.15) it fails and surfaces as "assign: Bad file descriptor" in reactive_descriptor_service, tearing down the IOTMP connection in a JOIN/LEAVE loop whenever the platform invokes a cmd. Disable pidfd via BOOST_PROCESS_V2_DISABLE_PIDFD_OPEN so the SIGCHLD handle is used everywhere, and wrap the cmd resource handler in a try/catch so any future throw from the execution path returns a 500 to the caller instead of escaping into the connection cycle.
1 parent d7db9b5 commit 94b570b

2 files changed

Lines changed: 71 additions & 51 deletions

File tree

CMakeLists.txt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,13 @@ else()
5050
endif()
5151
find_package(Boost REQUIRED COMPONENTS program_options date_time process iostreams)
5252

53+
# Boost.Process v2 defaults to pidfd_open on Linux for process tracking, but
54+
# that syscall requires kernel 5.3+. On older kernels (e.g. Ubuntu 18.04's
55+
# 4.15) it fails with ENOSYS and surfaces as "assign: Bad file descriptor"
56+
# from reactive_descriptor_service, taking down the caller. Force the
57+
# SIGCHLD-based handle so the binary stays portable to pre-5.3 kernels.
58+
add_definitions(-DBOOST_PROCESS_V2_DISABLE_PIDFD_OPEN)
59+
5360
add_definitions( -DTHINGER_ENABLE_STREAM_LISTENER)
5461
add_definitions( -DTHINGER_SERVER="iot.thinger.io")
5562
add_definitions( -DTHINGER_KEEP_ALIVE_SECONDS=60)

src/thinger/iotmp/extensions/cmd/cmd.cpp

Lines changed: 64 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -27,61 +27,74 @@ namespace thinger::iotmp{
2727
out["stdout"] = "";
2828
out["stderr"] = "";
2929
}else{
30-
std::string pout;
31-
std::string perr;
32-
auto command = get_value(in.payload(), "cmd", empty::string);
33-
auto timeout = get_value(in.payload(), "timeout", 30);
34-
auto stdin_data = get_value(in.payload(), "stdin", empty::string);
35-
bool timeout_flag = false;
36-
int retcode = 0;
37-
38-
// If the body starts with a shebang, materialize it to a temp
39-
// file and execute it directly so the interpreter declared in
40-
// the shebang (python3, node, …) takes effect. Otherwise fall
41-
// back to running the command through the shell, which keeps
42-
// multi-line shell snippets, pipes and redirects working.
43-
if(command.rfind("#!", 0) == 0){
44-
namespace fs = std::filesystem;
45-
std::random_device rd;
46-
std::uniform_int_distribution<uint64_t> dist;
47-
char buf[17];
48-
std::snprintf(buf, sizeof(buf), "%016llx", (unsigned long long) dist(rd));
49-
auto tmp_path = fs::temp_directory_path() / (std::string("thinr-cmd-") + buf);
50-
std::error_code ec;
51-
52-
{
53-
std::ofstream ofs(tmp_path, std::ios::binary);
54-
ofs.write(command.data(), command.size());
30+
// Isolate the entire execution from the caller's message loop:
31+
// Boost.Process v2, pipe setup and shell materialization can
32+
// throw (e.g. pidfd_open ENOSYS on pre-5.3 kernels surfaces
33+
// as "assign: Bad file descriptor"), and we must never let
34+
// those escape and tear down the IOTMP connection cycle.
35+
try{
36+
std::string pout;
37+
std::string perr;
38+
auto command = get_value(in.payload(), "cmd", empty::string);
39+
auto timeout = get_value(in.payload(), "timeout", 30);
40+
auto stdin_data = get_value(in.payload(), "stdin", empty::string);
41+
bool timeout_flag = false;
42+
int retcode = 0;
43+
44+
if(command.rfind("#!", 0) == 0){
45+
namespace fs = std::filesystem;
46+
std::random_device rd;
47+
std::uniform_int_distribution<uint64_t> dist;
48+
char buf[17];
49+
std::snprintf(buf, sizeof(buf), "%016llx", (unsigned long long) dist(rd));
50+
auto tmp_path = fs::temp_directory_path() / (std::string("thinr-cmd-") + buf);
51+
std::error_code ec;
52+
53+
{
54+
std::ofstream ofs(tmp_path, std::ios::binary);
55+
ofs.write(command.data(), command.size());
56+
}
57+
fs::permissions(tmp_path,
58+
fs::perms::owner_read | fs::perms::owner_write | fs::perms::owner_exec,
59+
fs::perm_options::replace, ec);
60+
61+
retcode = exec(tmp_path.string(), {}, pout, perr, stdin_data, timeout, &timeout_flag);
62+
63+
fs::remove(tmp_path, ec);
64+
}else{
65+
retcode = exec(preferred_shell(), {"-c", command}, pout, perr, stdin_data, timeout, &timeout_flag);
5566
}
56-
fs::permissions(tmp_path,
57-
fs::perms::owner_read | fs::perms::owner_write | fs::perms::owner_exec,
58-
fs::perm_options::replace, ec);
59-
60-
retcode = exec(tmp_path.string(), {}, pout, perr, stdin_data, timeout, &timeout_flag);
61-
62-
fs::remove(tmp_path, ec);
63-
}else{
64-
retcode = exec(preferred_shell(), {"-c", command}, pout, perr, stdin_data, timeout, &timeout_flag);
65-
}
6667

67-
// signal error at protocol level
68-
if(timeout_flag){
69-
out.set_error(408, "command timed out");
70-
}else if(retcode != 0){
71-
out.set_error(500, "command failed");
72-
}
68+
if(timeout_flag){
69+
out.set_error(408, "command timed out");
70+
}else if(retcode != 0){
71+
out.set_error(500, "command failed");
72+
}
7373

74-
std::string mode = get_value(in.payload(), "mode", empty::string);
75-
if(mode=="api" || mode == ""){
76-
out["retcode"] = retcode;
77-
out["stdout"] = pout;
78-
out["stderr"] = perr;
79-
}else if(mode=="text"){
80-
if(!pout.empty()){
81-
out = pout;
82-
}else{
83-
out = perr;
74+
std::string mode = get_value(in.payload(), "mode", empty::string);
75+
if(mode=="api" || mode == ""){
76+
out["retcode"] = retcode;
77+
out["stdout"] = pout;
78+
out["stderr"] = perr;
79+
}else if(mode=="text"){
80+
if(!pout.empty()){
81+
out = pout;
82+
}else{
83+
out = perr;
84+
}
8485
}
86+
}catch(const std::exception& e){
87+
LOG_ERROR("cmd execution failed: {}", e.what());
88+
out.set_error(500, std::string("command execution error: ") + e.what());
89+
out["retcode"] = -1;
90+
out["stdout"] = "";
91+
out["stderr"] = e.what();
92+
}catch(...){
93+
LOG_ERROR("cmd execution failed: unknown exception");
94+
out.set_error(500, "command execution error: unknown");
95+
out["retcode"] = -1;
96+
out["stdout"] = "";
97+
out["stderr"] = "unknown exception";
8598
}
8699
}
87100
};

0 commit comments

Comments
 (0)