diff --git a/src/doompanning.cc b/src/doompanning.cc index 91ddbab..5a9dbde 100644 --- a/src/doompanning.cc +++ b/src/doompanning.cc @@ -166,45 +166,52 @@ inline auto dp_elapsed(const std::chrono::steady_clock::time_point &tStart) { re void check_on_dooms(ControllerContext &ctx) { - #if 0 pid_t pid = 0; - // This works for dooms forked from the controller, not for externally - // started dooms. + // Detect dooms that have terminated. This works for dooms forked from the + // controller, not for externally started dooms. do { int wstatus = 0; if (pid = waitpid(0, &wstatus, WNOHANG); pid > 0) { - auto ds = find_in_container(ctx.dooms, [pid] (const auto &ds) { return ds.id == pid; }); - - assert(ds != std::end(ctx.dooms)); - - if (ds != std::end(ctx.dooms)) + if (auto ds = find_in_container(ctx.dooms, [pid] (const auto &ds) { return ds.id == pid; }); + ds != std::end(ctx.dooms)) { if (WIFEXITED(wstatus)) log_info("doom(%d) exited with status %d", pid, WEXITSTATUS(wstatus)); else if (WIFSIGNALED(wstatus)) log_warn("doom#(%d) got killed by signal %d", pid, WTERMSIG(wstatus)); - SDL_DestroyTexture(ds->texture); // TODO: use the destructor to do this - ctx.dooms.erase(ds); + // Manually set Endoom state and let the code below clean it up. + ds->state = DP_DS_Endoom; } } } while (pid > 0); - #else - const auto sz0 = ctx.dooms.size(); - auto eb = std::remove_if(std::begin(ctx.dooms), std::end(ctx.dooms), [] (const auto &ds) { return ds.state == DP_DS_Endoom; }); - if (eb != std::end(ctx.dooms)) + + // Find dooms that are in Endoom state and remove them. This works for + // externally started dooms if we received their Endoom DoomState update. { - auto count = std::distance(eb, std::end(ctx.dooms)); - std::for_each(eb, std::end(ctx.dooms), [] (auto &ds) { SDL_DestroyTexture(ds.texture); ds.texture = nullptr; }); - ctx.dooms.erase(eb, std::end(ctx.dooms)); - const auto sz1 = ctx.dooms.size(); - log_info("Erased %zu dooms which were in Endoom state. doomcount before=%zu, after=%zu", count, sz0, sz1); + const auto prevCount = ctx.dooms.size(); + const auto removedBegin = std::remove_if(std::begin(ctx.dooms), std::end(ctx.dooms), [](const auto &ds) + { return ds.state == DP_DS_Endoom; }); + if (removedBegin != std::end(ctx.dooms)) + { + auto count = std::distance(removedBegin, std::end(ctx.dooms)); + std::for_each(removedBegin, std::end(ctx.dooms), [](auto &ds) + { SDL_DestroyTexture(ds.texture); ds.texture = nullptr; }); + ctx.dooms.erase(removedBegin, std::end(ctx.dooms)); + const auto newCount = ctx.dooms.size(); + log_info("Erased %zu dooms which were in Endoom state. Doomcount before=%zu, after=%zu", count, prevCount, newCount); + } } - #endif + + // FIXME: We can miss Endoom state updates when nng has to drop message due + // to queue size limits. If this happens for an externally started doom it + // will never be removed from ctx.dooms (waitpid() does not work because the + // doom is not our child). Use DoomState::tLastActive and a fixed timeout + // value to timeout dooms and erase them from ctx.dooms. } void do_networking(ControllerContext &ctx)