Address race condition and timeout issue in docker engine http client, Docker holds the /stop response until the container stops or the grace period expires.

This commit is contained in:
mrkmntal 2026-08-29 12:38:23 -04:00
commit 0d5ee33e75
3 changed files with 26 additions and 42 deletions

View file

@ -34,7 +34,8 @@ bool DockerEngineClient::checkConnection(std::string& error) const {
EngineResponse DockerEngineClient::request(const std::string& method,
const std::string& path,
const std::string& body) const {
const std::string& body,
int timeout_seconds) const {
EngineResponse response;
const int fd = socket(AF_UNIX, SOCK_STREAM, 0);
if (fd == -1) {
@ -57,7 +58,7 @@ EngineResponse DockerEngineClient::request(const std::string& method,
}
struct timeval timeout{};
timeout.tv_sec = 10;
timeout.tv_sec = timeout_seconds;
setsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, &timeout, sizeof(timeout));
setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof(timeout));
@ -88,7 +89,7 @@ EngineResponse DockerEngineClient::request(const std::string& method,
}
close(fd);
if (count < 0) {
if (errno == EINTR) return request(method, path, body);
if (errno == EINTR) return request(method, path, body, timeout_seconds);
if (errno != EAGAIN && errno != EWOULDBLOCK) {
response.error = std::strerror(errno);
return response;

View file

@ -18,7 +18,8 @@ public:
EngineResponse request(const std::string& method,
const std::string& path,
const std::string& body = {}) const;
const std::string& body = {},
int timeout_seconds = 10) const;
private:
std::string socket_path_;

View file

@ -160,49 +160,31 @@ bool DockerManager::deleteImage(const std::string& id, std::string& message) con
bool DockerManager::stopContainer(const std::string& id,
std::string& message,
int timeout_seconds) const {
const auto stop_request = [this, &id, timeout_seconds] {
return engine_.request("POST", "/containers/" + id + "/stop?t=" + std::to_string(timeout_seconds));
};
const auto acceptable = [](const EngineResponse& response) {
return response.status_code == 204 || response.status_code == 304 || response.status_code == 404 || response.ok();
};
const auto probe = [this, &id] {
const EngineResponse state = engine_.request("GET", "/containers/" + id + "/json");
if (!state.ok()) return StopProbe::Unknown;
try {
const bool running = nlohmann::json::parse(state.body)
.value("State", nlohmann::json::object())
.value("Running", true);
return running ? StopProbe::Running : StopProbe::Stopped;
} catch (const nlohmann::json::exception&) {
return StopProbe::Unknown;
}
};
const EngineResponse first = stop_request();
const bool first_timed_out = first.error == "Docker Engine response timed out.";
if (!acceptable(first) && !first_timed_out) {
message = apiError(first, "Could not stop that container.");
if (id.empty()) {
message = "Please provide a container ID.";
return false;
}
if (timeout_seconds < 0) {
message = "Stop timeout cannot be negative.";
return false;
}
StopProbe state = first_timed_out ? StopProbe::Unknown : waitForStopped(probe);
if (state != StopProbe::Stopped) {
const EngineResponse retry = stop_request();
const bool retry_timed_out = retry.error == "Docker Engine response timed out.";
if (!acceptable(retry) && !retry_timed_out) {
message = apiError(retry, "Could not confirm that container stopped.");
return false;
}
state = waitForStopped(probe);
// Docker holds the /stop response until the container stops or the grace
// period expires. Use a per-request HTTP timeout longer than the grace
// period so the client does not race Docker's own timeout.
const EngineResponse response = engine_.request(
"POST",
"/containers/" + id + "/stop?t=" + std::to_string(timeout_seconds),
"",
timeout_seconds + 5);
if (response.ok() || response.status_code == 304 || response.status_code == 404) {
message = "Container stopped.";
return true;
}
if (state != StopProbe::Stopped) {
message = "Stop requested, but container state could not be confirmed.";
return false;
}
message = "Container stopped.";
return true;
message = apiError(response, "Could not stop that container.");
return false;
}
bool DockerManager::removeContainer(const std::string& id, std::string& message) const {