nut-scanner: retry SNMP session opens after descriptor exhaustion by user01010111 · Pull Request #3673 · networkupstools/nut

user01010111 · 2026-09-20T04:18:02Z

Depends on #3649. I am opening this as a draft while that PR is unmerged.
The branch includes its four existing commits followed by this four-file
SNMP recovery change.

I retain SNMP targets whose session open fails with EMFILE and retry them one
at a time after the original SNMP scans finish. If a serial attempt still runs
out of descriptors, I report that host and stop retrying it. Other session-open
errors keep their existing handling.

This follows PR #3649 and partially addresses #2576. It uses the existing
admission helpers and does not adjust shared thread limits or other scan types.

I tested the actual scanner worker in an instrumented harness with NetSNMP
5.9.5.2 and a loopback SNMP responder. The baseline missed all five targets under
controlled descriptor exhaustion; this change found all five exactly once after
descriptors were released. Persistent exhaustion terminated with a warning per
host and balanced descriptor and permit counts.

I also ran 124 focused scenarios across semaphore, counter-only and no-thread
Linux builds with address, undefined-behaviour and leak sanitizers. The GNU89
build, project checks and distcheck-light passed, including 16/16 test
programmes and staged install/uninstall. Documentation rendering was disabled;
the light distribution check used its standard placeholder manual pages.

AI assistance: OpenAI Codex, including a focused review of worker ownership and
admission accounting.

Skip inactive protocol semaphores, unwind partial admission, and return owned permits when worker allocation or creation fails. Allocate tracking entries before starting workers and keep NUT hostname and authentication cleanup with the worker until it finishes. Add offline regression tests using real threads and semaphores, preserve the counter and non-threaded paths, and record the change in NEWS. Related to networkupstools#1166. Global semaphore initialisation policy remains separate. Prepared with assistance from OpenAI Codex (gpt-6-astra). Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com>

Pass the configured SSL include flags to the scanner tests for NSS builds. Order scanner recursion after common and client dependencies, and generate the SNMP header before compiling the test source. Prepared with assistance from OpenAI Codex (gpt-6-astra). Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com>

Track the protocol semaphore only after successful initialisation, and check destruction or close before clearing fixture state. Assert clean state at scenario entry and clear the global semaphore after teardown. Assisted by OpenAI Codex (gpt-6-astra). Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com>

Move the shared worker helpers into nutscan-thread.c and keep declarations in the header. Preserve internal symbol visibility and the existing failure-injection tests. Assisted by OpenAI Codex (gpt-6-astra). Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com>

Retain SNMP hosts whose session open fails with EMFILE and retry them serially after the original workers finish. Report persistent exhaustion without requeueing, preserving admission accounting and target ownership. Add focused recovery tests and a NEWS entry. This partially addresses issue networkupstools#2576 and follows the admission helpers from PR networkupstools#3649. Assisted by OpenAI Codex. Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com>

github-actions · 2026-09-20T04:19:04Z

A ZIP file with standard source tarball and another tarball with pre-built docs for commit d961bb1 is temporarily available: NUT-tarballs-PR-3673.zip.

+	void *arg;
+};
+
+static void *offline_worker(void *opaque)


Keep test setup, scanner execution and checks active when built with -DNDEBUG. Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com>

Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com>

…e other out-of-dir stuff [networkupstools#3673] Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>

…ode [networkupstools#3673] Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>

… acquire [networkupstools#3673] Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>

…arg() for simplicity [networkupstools#3673] Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>

…safe side to cast it as (unsigned int) [networkupstools#3673] ...This may get revisited after we consistently use one boolean type in the code base. Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>

jimklimov

Posted a few specific questions here, mostly about erring on the safe side while working with memory, and will post some casual fixes to the code too.

General questions:

Why is this refactor not applied to serial port scanning? IIRC it used semaphores too?..
Why is the fallback (to follow up with overflowing IP addresses) only applied to SNMP and not other protocols? The idea does seem sane...

jimklimov · 2026-09-21T18:05:25Z

Was there a particular reason to remove this (opportunistic) cleanup? Did it break something? Do we potentially lose something with it dropped?

The removed branch posted permits for entries marked inactive. The refactor records only successfully created workers; failed creation releases its admission permits without adding a tracked entry. Successful workers release their permits when joined. An inactive entry therefore has no outstanding permit to release here, and normal permit cleanup still happens through the join path.

jimklimov · 2026-09-21T18:07:17Z

 			} /* if: could we "pass" or not? */
 		} /* while */

+		free(ip_str);


Shouldn't we set ip_str=NULL here too, erring on the safe side?

Added ip_str = NULL after the free in f9269a1.

jimklimov · 2026-09-21T18:10:12Z

+#if defined HAVE_PTHREAD && (defined HAVE_SEMAPHORE_UNNAMED || defined HAVE_SEMAPHORE_NAMED)
+				nut_scanner_semaphore_release(semaphore, semaphore_scantype, max_threads_scantype);
+#endif
 				free(ip_dest);


Probably should safely set ip_dest=NULL here...

Added ip_dest = NULL after the free in f9269a1.

jimklimov · 2026-09-21T18:10:53Z

+				int ret = nut_scanner_thread_create(&thread_array, &thread_count,
+					list_nut_devices_thready, (void *)nut_arg);
+				if (ret != 0) {
+					free_nut_arg(nut_arg);


Set nut_arg=NULL here?

Added nut_arg = NULL after cleanup in f9269a1.

jimklimov · 2026-09-21T18:12:01Z

+	if (nut_arg) {
+		free_nut_arg(nut_arg);
+	}



Set nut_arg=NULL here to err on the safe side? Maybe propagate it as ** pointer so all owners/observers end up with NULL when they should?

Added nut_arg = NULL at both cleanup callers in f9269a1 and retained your NULL-input guard. I kept free_nut_arg() taking the pointer by value: ownership transfers to the worker when thread creation succeeds, and the dispatcher handles cleanup if creation fails. Each owner now clears its local pointer after cleanup. A ** parameter would clear the supplied pointer, but would not clear other aliases.

jimklimov · 2026-09-21T18:15:26Z

 		} /* if: could we "pass" or not? */
 	} /* while */

+	free(ip_str);


Shouldn't we set ip_str=NULL here too, erring on the safe side?

Added ip_str = NULL after the free in f9269a1.

jimklimov · 2026-09-21T18:16:27Z

 			} /* if: could we "pass" or not? */
 		} /* while */

+		free(ip_str);


Shouldn't we set ip_str=NULL here too, erring on the safe side?

Added ip_str = NULL after the free in f9269a1.

Clear local pointers after releasing scanner-owned storage. Assisted by OpenAI Codex. Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com>

Reuse the shared admission and thread helpers for serial scans, release permits when worker creation fails, and join workers before freeing their borrowed port names or destroying the device mutex. Extend the existing offline admission test for serial scans and record the change in NEWS. Assisted by OpenAI Codex. Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com>

Link the serial archive before the common libraries that supply its logging and Windows serial helpers. This fixes the unresolved symbols when linking nutscan-serial-test with shared private libraries on MinGW. Assisted by OpenAI Codex (GPT-6). Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com>

user01010111 · 2026-09-28T04:36:25Z

Serial scanning was an omission. Commit 6272293 now uses the existing admission helpers there, allocates tracking before starting workers, releases permits when thread creation fails, and keeps borrowed port names alive until all workers are joined.

I have kept descriptor-exhaustion recovery limited to SNMP because that is the session-open failure path reproduced and tested here. Extending it to the other scanner backends needs validation of each library's error reporting and cleanup before retrying. This remains a partial fix for #2576.

Carry the common scanner-test Net-SNMP executable flags and SSL dependency so static archive references resolve for every fixture, including retries. Retain the original library passthrough flags and cache both forms through the existing dependency macro. Keep both fixture mutexes separate from the library and order common-library sub-makes to prevent races over shared replacement-function objects. Validated static scanner and retry tests on Linux and native MinGW Windows, clean two-job builds, and distcheck-light with install/uninstall. Prepared with OpenAI Codex (GPT-6) assistance, following the earlier Claude Opus 5.5 review. Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com>

user01010111 · 2026-10-14T02:19:03Z

The scanner and retry tests pass against static libnutscan on Linux and MinGW Windows, including clean parallel builds with two jobs. This carries the corrected #3649 common test-link rule, common-library build ordering and SSL dependency, so the retry target receives the static Net-SNMP libraries as well. Both fixtures keep their mutex separate from the library's definition. The distribution and install/uninstall checks pass.

Prepared with Codex assistance, following the earlier Claude Opus 5.5 review.

AppVeyorBot · 2026-10-24T09:00:16Z

✅ Build nut 2.8.5.5897-master completed (commit d154405169 by @jimklimov)

artifacts
A 7-zip archive with built NUT for Windows debug binaries is temporarily available: NUT-for-Windows-x86_64-SNAPSHOT-2.8.5.5897-master.7z
ccache: 30 of 584 compilations were cache hits (5.1%); cache size 749 of 750 MiB after cleanup

user01010111 added 5 commits September 15, 2026 00:26

github-advanced-security Bot found potential problems Sep 20, 2026

View reviewed changes

user01010111 added 2 commits September 20, 2026 05:09

tests: keep scanner assertions enabled with NDEBUG

76a607b

Keep test setup, scanner execution and checks active when built with -DNDEBUG. Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com>

tests: initialise Winsock for SNMP retry checks

d008380

Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com>

jimklimov added nut-scanner SNMP AI For good or bad, machine tools are upon us. Humans are still the responsible ones. portability We want NUT to build and run everywhere possible labels Sep 21, 2026

jimklimov added this to the 2.8.6 milestone Sep 21, 2026

jimklimov added 6 commits September 21, 2026 15:10

tests/Makefile.am: group nut-scanner related dependencies on top, lik…

c84b01a

…e other out-of-dir stuff [networkupstools#3673] Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>

tools/nut-scanner/nutscan-thread.c: style fixes [networkupstools#3673]

c78dfd0

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>

tools/nut-scanner/nutscan-thread.{c,h}: update (C) for transplanted c…

5baa9c3

…ode [networkupstools#3673] Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>

tools/nut-scanner/scan_*.c: restore debug-log after successful thread…

a01ccb4

… acquire [networkupstools#3673] Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>

tools/nut-scanner/scan_nut.c: move nut_arg==NULL check into free_nut_…

20e2173

…arg() for simplicity [networkupstools#3673] Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>

tools/nut-scanner/scan_*.c: when logging "pass" (bool_t), err on the …

827e2cb

…safe side to cast it as (unsigned int) [networkupstools#3673] ...This may get revisited after we consistently use one boolean type in the code base. Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>

jimklimov requested changes Sep 21, 2026

View reviewed changes

jimklimov added the refactor/fightwarn PR or issue proposal to improve code maintainability without functional changes, or to fix warnings label Sep 21, 2026

user01010111 added 2 commits September 21, 2026 22:06

nut-scanner: clear freed scanner locals

f9269a1

Clear local pointers after releasing scanner-owned storage. Assisted by OpenAI Codex. Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com>

user01010111 requested a review from jimklimov September 21, 2026 22:14

github-advanced-security Bot found potential problems Sep 21, 2026

View reviewed changes

Comment thread tests/nutscan-thread-test.c Dismissed

Comment thread tests/nutscan-thread-test.c Dismissed

Comment thread tests/nutscan-thread-test.c Dismissed

Comment thread tests/nutscan-thread-test.c Dismissed

Comment thread tests/nutscan-thread-test.c Dismissed

Merge branch 'master' into fix/snmp-emfile-recovery

d31b3ac

user01010111 mentioned this pull request Sep 23, 2026

nut-scanner: continue scans after global semaphore initialisation fails #3707

Open

user01010111 marked this pull request as ready for review October 12, 2026 08:26

Merge branch 'master' into fix/snmp-emfile-recovery

9d9686f

github-actions Bot deleted a comment from AppVeyorBot Oct 14, 2026

Merge branch 'master' into fix/snmp-emfile-recovery

d961bb1

github-actions Bot deleted a comment from AppVeyorBot Oct 23, 2026

Conversation

user01010111 commented Sep 20, 2026

Uh oh!

github-actions Bot commented Sep 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jimklimov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

user01010111 commented Sep 28, 2026

Uh oh!

user01010111 commented Oct 14, 2026

Uh oh!

AppVeyorBot commented Oct 24, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Sep 20, 2026 •

edited

Loading