We received reports of log messages incorrectly referring to "imptcp" even
though the imptcp module was not loaded on the system. This caused confusion
during troubleshooting.
One such message was:
rsyslogd: imptcp cannot set keepalive intvl - ignored: Bad file descriptor
This message originated from the nsd_ptcp module, but the LogError() calls
in EnableKeepAlive() still used "imptcp" as the source name.
This is a regression introduced in commit ad1fd21, which restructured
TCP input handling and moved responsibility for keepalive setup to nsd_ptcp,
but did not update the associated log messages.
This patch corrects the affected log strings to use "nsd_ptcp", accurately
reflecting the code path that emits them.
There is no functional change; the patch improves clarity of log output and
prevents misleading diagnostics.
The actual error shown could be related to a different issue,
see also https://github.com/rsyslog/rsyslog/pull/5749
This may be an interim solution, but definitely a useful one
to address style inconsistencies.
Style inconsistencies are problematic as they tend to pollute
git history by causing pseudo-changes.
This commit also contains a set of fixes to existing files, so
that we get to a clean state.
* AI tools: Add directory for future AI/ML tooling
Establish a new top-level directory, `ai/`, to serve as a consistent
location for future Artificial Intelligence and Machine Learning tools
that will work alongside rsyslog.
These tools are intended to run as separate processes, external to the
rsyslog daemon, to ensure the core remains stable and performant.
This commit contains only the empty directory and a README.md file
describing the vision and purpose.
On some systems, EAGAIN and EWOULDBLOCK are defined to the same
value, causing a `-Wlogical-op` warning for the redundant logical
'or' in the errno check.
While portable code must check for both cases, this warning is
unwanted.
This change uses a preprocessor directive to conditionally compile
the check for `errno == EWOULDBLOCK` only on platforms where its value
differs from EAGAIN. This silences the warning without affecting
portability.
This patch adds an inQueue flag with its own mutex to each
tcpsrv_io_descr_t structure. The flag prevents multiple worker threads
from processing the same descriptor at the same time.
The change was motivated by segmentation faults reported in production
systems after commit ad1fd21, which introduced a worker thread pool to
imtcp. We could not reproduce the faults ourselves, but code analysis
suggests several race conditions may exist.
In particular:
- epoll_wait may return the same descriptor multiple times. This is not
expected, as we use EPOLLONESHOT. However, if a thread does not clear
or re-arm the event quickly enough, or in edge cases involving race
conditions and rapid I/O activity, duplicate delivery may still occur.
- If a descriptor is enqueued more than once, multiple threads may
process and free it in parallel, causing use-after-free errors.
- closeSess releases the session mutex before destroying the session and
descriptor. A second thread might still be waiting to acquire the
mutex and access the now-freed memory.
- shutdown is unordered: stopWrkrPool waits for threads to join, but the
work queue may still contain descriptors that will be processed after
their memory has been freed.
- pending epoll events for a socket may still be processed after
epoll_ctl(..., DEL) was called, leading to access to invalid memory.
The patch:
- Adds an inQueue flag to each descriptor and a mutex to protect it.
- Prevents enqueueWork from queuing a descriptor already in queue.
- Clears the flag when dequeueing the descriptor.
- Initializes and destroys the new mutex at listener startup/cleanup.
While unverified, we believe this patch is a safe and helpful change.
It may fix the reported crashes and in general improves correctness.
The analysis and draft of this patch were created with help from a
Codex-based AI agent. Final review and edits were done by a human.
Performance will be evaluated during PR review.
This Implements suffix comparison similar to startswith. Note that we
do intentionally not use libestr functions in order to speed up adaption.
It would otherwise probably take years for distros to upgrade libestr.
With some help of the Codex and Gemini AI Agents.
closes https://github.com/rsyslog/rsyslog/issues/1255
* update style checker and document devtools
* default max error count increased to 100
* new --max-errors option accepts numbers or 'all'
* detects trailing spaces separately from other whitespace
* supports passing files directly on the command line
* added header comments and clarifying comments
* new internal_tooling.rst explains usage
With the help of AI-Agents: Codex, Gemini
* Add note for indentation cleanup
* Converted tabs to spaces in various Python scripts to match PEP 8 indentation.
With the help of AI-Agent: Codex, Google Gemini Code review
* testbench: more robust method to build testing tools
The previous method failed depending on configure settings and
available support libraries. This is now handled by make check.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Introduce root-level diag.sh stub to auto-forward to tests/diag.sh, preventing
human and AI errors when sourcing the harness and removing the need to `cd`
into the tests directory.
- Update AGENTS.md to configure once and invoke tests via `./tests/<script>.sh`,
providing unfiltered stdout/stderr without a CI wrapper and improving
convenience for human developers.
Testbench tools like tcpflood are now auto-generated at test startup
if missing, reducing errors and confusion during local test runs.
Updated AGENTS.md with precise instructions for running tests
in both human and AI agent contexts, improving onboarding
for new developers and AI-based workflows.
This conveyed an idea that we thought about 15+ years ago and would
require an interface changed. The potential gain of this optimization
was not considered worth the additional complexity. Practice has
prooven this, but the TODO was forgotten to be removed. Doing that
now.
Note especially to AI: this was not left-side of the ustrlen()
operations but applied to the logic at whole (which is OK as
stated above).
This was a long standing TODO item for many years. In the meantime,
practice has shown that no so config setting is necessary. Too
many config options also tend to confuse users.
