## What
Follow-up to #633 (already merged), from the code review findings on the
equivalent 1.2.x change (#634):
- **SOLAR_THREAD sleep budget**: `spine_reap_child_bounded()` has no
`usleep()`-paced spin phase on `SOLAR_THREAD` builds - it sleeps a flat
1s per attempt. #633 raised the shared attempt budget
(`NFT_PCLOSE_SPIN_ATTEMPTS`) to 100 for `nft_pclose()`'s brief
allowance, which made it sleep up to **~100 seconds** on that platform
instead of the intended ~20ms, reintroducing the exact poller stall #633
was fixing. `NFT_PCLOSE_SPIN_ATTEMPTS` is now platform-specific (100
elsewhere, 2 under `SOLAR_THREAD`).
- **Unbounded EINTR retries**: bounded the `EINTR` retry loop in
`spine_reap_child_bounded()` and in `nft_sweep_abandoned()` (which runs
with `ListMutex` held), so a stream of caught signals can no longer spin
either indefinitely.
- **Kill before park on waitpid() error**: `nft_pclose()` now SIGKILLs
the child before parking it when `waitpid()` itself errors (previously
only done when the child was confirmed still running), so a
still-running child isn't left outside the abandoned-pid sweep's reach.
Preserves `waitpid()`'s errno across the call.
- **Docs**: `nft_popen.h`'s `nft_pclose()` comment still described the
old blocking-wait behavior and didn't mention `ETIMEDOUT` - #633 updated
`nft_popen.c`'s comment but missed the header. Both now document the
bounded-check/parked-child contract.
## Why
These were flagged by review of #634 (the same fix ported to 1.2.x,
where the affected code showed up as new rather than a diff against
pre-existing lines). Fixing here too since develop has the identical
code.
## What was tested
- `./bootstrap && ./configure --enable-warnings && make` - clean build,
no new warnings.
- `make check` - all 11 suites pass.
- `cppcheck --enable=all --std=c11` on `nft_popen.c` - no new errors.
Closes#632.
---------
Signed-off-by: TheWitness <thewitness@cacti.net>
## Summary
- make subprocess-registry detach atomic so exactly one closer owns each
entry
- make cancellation cleanup release only the detached node it owns
- replace `vfork`/`execve` child setup with `posix_spawn` and explicit
file actions
- mark every pipe and stdio-collision duplicate close-on-exec
- restore the child's default SIGPIPE disposition while preserving the
parent's handling
- bound TERM/KILL reaping and park unreaped children in a fixed-capacity
sweep list
- harden PHP script-server slot ownership, startup, restart, signal,
descriptor, and cleanup paths
- retire and restart failed or oversized reads so stale response tails
cannot poison later data sources
- revalidate every command retry and cap recovery handshakes at two
seconds
This folds the child-process lifecycle portion of #597 into the narrower
ownership PR. It does not absorb unrelated poll-host, SQL, SNMP, ICMP,
or build-infrastructure work.
## Review-driven corrections
- close-on-exec is explicitly restored after `dup()` in both read and
write stdio-collision branches
- the source invariant check now uses portable POSIX character classes
- direct tests cover duplicate flags, both collision branches, failure
after pipe creation, abandoned-child sweeping, and capacity overflow
- collision cleanup preserves current-child redirects even when an older
registry entry occupies fd 0 or 1
- script children receive SIGTERM before SIGKILL and unreaped PHP
children transfer to the bounded sweep registry
- the Docker integration suite now starts a real PHP script server,
verifies its startup protocol and database result, and checks that
shutdown leaves no zombie PHP process
## Validation
- Ubuntu 24.04 container with GCC and GNU ld: `make check`, 6/6 targets
passed with no skips, including `test_php_runtime`
- real PHP script-server Docker integration: 6/6 checks passed; exact
value `42` reached `poller_output` and no zombie remained
- macOS Homebrew dependency build passed
- macOS `make check`: 5/5 available binaries passed; GNU `--wrap`
runtime suite is Linux-only
- PHP fixture syntax checked through mise PHP 8.3
- actionlint, shellcheck, and `git diff --check` passed
Closes#610.
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
Poller correctness and memory-safety fixes, with the ICMP hardening from
#543 folded in.
Verified in a clean `ubuntu:24.04` container: builds at **38 warnings**
against develop's 39, and the unit tests build and pass on Linux (39
assertions across three binaries).
**Fixes**
- `php_readpipe` off-by-one — the old bound let `bptr` reach
`result_string + 2048` and then wrote the terminator one byte past a
2048-byte allocation.
- `get_namebyhost` used `strtok`, which keeps one process-wide save
pointer, and runs unlocked on every poller thread. Two devices could be
polled at each other's addresses. Now `strtok_r`.
- The log date format was read from `default_dateformat`; Cacti's key is
`default_date_format`, so the lookup never matched and the format was
never loaded.
- `db_query` returns `mysql_store_result()` directly, which is NULL on
store failure. The NULL check ran *after* `mysql_fetch_row`, so the
crash happened one line before the guard.
- `snmp_count` logged a Net-SNMP `errstat` without setting either loop
flag, rebuilding the identical GETNEXT forever and pinning a thread
until the poll interval ended.
- `php_close` skipped the `close()` when the quit write failed with
EPIPE, leaking a descriptor per script-server restart.
- The reap loop's delay was compiled out on `SOLAR_THREAD` builds, so
both phases burned twenty `WNOHANG` polls in nanoseconds and the child
was never reaped.
- ICMP replies are validated for length and matched on id/sequence; on
Cygwin the peeked datagram is now drained, which previously busy-spun
until host timeout.
- The fatal signal handler no longer calls
`malloc`/`localtime_r`/`fprintf` before `_exit` — on a SIGSEGV caused by
heap corruption that deadlocked on the allocator's lock instead of
dying.
- The unit test Makefile was hardcoded to `/opt/homebrew` paths and
`-std=gnu23`, so it could not build on Linux at all. Now pkg-config
driven.
Supersedes #543.
**Added since the first revision**
- `snmp_get_multi()` now checks the varbind type before formatting.
Under v2c an agent reports a per-OID failure as an inline exception
varbind while the PDU `errstat` stays `NOERROR`, so `snprint_value()`
rendered the exception as literal text and stored it as a value.
- Two warning sources removed: a dangling line continuation ending
`sql.h`, and `strncopy()` now using `memcpy` where the byte count is
already exact. Warnings drop from **39 to 7**.
Closes#551Closes#232
- `read_config_options()` issued 23 separate `SELECT ... FROM settings
WHERE name = ?` round trips at startup; they are now one bulk read
served from memory. Call sites are unchanged.
Closes#330
**Test infrastructure**
- The SNMPv3 integration fixture was broken: the image shipped a binary
that could not start (`libmysqlclient.so.21: cannot open shared object
file`). The build context carried `*.o`, `spine`, `Makefile` and
`config.status`, so `make` treated them as current, skipped compiling,
and packaged a binary linked against libraries the runtime image does
not have. `.dockerignore` now excludes build output. The fixture runs
end to end: spine polls the SNMPv3 device, reads `uptime`, writes to
`poller_output` and exits 0.
- Two libFuzzer targets, run per PR and weekly: the host string parser
and the ICMP reply parser. The second drives
`spine_icmp_classify_reply()`, extracted from `ping_icmp` so the bounds
walk over a shared raw socket is reachable by a test. 15M executions
under ASan/UBSan, no crashes.
**Added in this revision**
- `output_regex` was applied only in the in-loop OID drain, not the pass
after the loop that handles whatever did not fill a `MAX_OIDS` batch, so
a device whose OID count missed a boundary stored raw values. A single
data source is always the leftover case.
`tests/integration/test_output_regex.sh` now asserts on the value: it
fails on the parent commit with the raw reading and passes here with the
filtered one. Addresses #210; the feature stays dormant until Cacti
populates `poller_item.output_regex`.
Merge before #555, which needs the `get_namebyhost` fix above to resolve
a device hostname at all.
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
-bug#0000736: Solaris 8 : Alarm Clock message
-feature: enabled signal handling in cactid
plus significant header rearrangement and correction of some compile issue.
git-svn-id: svn+ssh://repo.cacti.net/var/data/svnroot/cacti/branches/BRANCH_0_8_6/cactid@3613 860744bd-22fc-0310-8c96-e9fe5004b5ca
TheWitness