Skip to content

PyNUT: restore socket cleanup in disconnect() - #3627

Open
user01010111 wants to merge 3 commits into
networkupstools:masterfrom
user01010111:codex/fix-3619-pynut-disconnect
Open

PyNUT: restore socket cleanup in disconnect()#3627
user01010111 wants to merge 3 commits into
networkupstools:masterfrom
user01010111:codex/fix-3619-pynut-disconnect

Conversation

@user01010111

Copy link
Copy Markdown

Calling disconnect() on a retained PyNUTClient leaves its socket open:
the literal name passed to hasattr() does not undergo Python's private
name mangling. Read the existing class attribute directly so the existing
LOGOUT, close and reference cleanup code runs. Its class-level None
default covers deferred and partial initialization; send and close errors
remain independently suppressed.

Add 12 regression tests to the existing Python test script and run them
from make check, without requiring an external NUT server. They cover
cleanup order, repeat calls, explicit destructor calls, subclasses,
connection/authentication failures, send/close failures, and real loopback
transmission and closure. The same script continues to run the existing
server integration tests. Add a release-note entry; no public API,
dependency, version or hardware-support change is intended.

Closes: #3619

Validation:

  • The final regression suite fails 8/12 tests on unmodified upstream
    ab687a162ad99d2c687c48f02b498845f69c4d91, including absent LOGOUT/close
    events and the retained open socket. All 12 pass with the fix.
  • Linux ARM64: Python 2.6.9, 2.7.18, 3.5.10, 3.6.15, 3.7.17, 3.8.20,
    3.9.25, 3.10.21, 3.11.16, 3.12.14, 3.13.15 and 3.14.7 pass.
    macOS ARM64: Python 3.9.6 and 3.14.7 pass.
  • Supplementary IPv6, verified localhost TLS, failed STARTTLS setup and
    cipher-fallback cleanup tests pass on Linux and macOS.
  • Locally built upsd and dummy-ups: retained plain/TLS clients release
    their sockets and logged-in sessions after explicit/destructor cleanup;
    replacement clients reconnect. The existing Python script passes on
    Python 2.6.9 and 3.11.15. The NUT Python integration harness passes
    4 groups with no failures or skips.
  • Generated modules, locally installed wheel and source distribution,
    and staged NUT install pass. Source-package installation and its installed
    tests also pass on Python 2.6.9 using setuptools 36.8.0.
  • The broad Linux build, make check (including 12 Automake tests),
    make stylecheck, make spellcheck and make distcheck-light pass.
    The final distribution check generates real man pages and HTML and
    exercises the extracted archive's build, tests, install/uninstall and
    cleanup. The four changed source files match that archive exactly.

No physical UPS hardware was available for testing. Validation covered
deterministic socket tests, real localhost TCP/TLS connections, and locally
built upsd with simulated dummy-ups devices.
Physical UPS models, firmware versions, and hardware combinations were
not tested.

Windows was not executed. Python 2.6 uses the plain socket path because
this module disables SSL on runtimes without ssl.create_default_context.

Relevant checklist:

  • Concrete behavior, compatibility expectations and limitations described.
  • Focused change; existing templates and test/distribution infrastructure used.
  • New source comments and release-note text use ASCII.
  • NEWS entry added; no driver, manual/API or UPGRADING change required.
  • Build, tests, package/install, spelling/style and distcheck-light pass.
  • AI use and model disclosed.
  • Human contributor review and acceptance of responsibility completed.

AI assistance: OpenAI Codex with gpt-6-astra (x-high reasoning).
The human contributor remains responsible for reviewing and submitting
the change.

Remove the unmangled hasattr() check which prevented cleanup of a
retained client's socket. The class-level None default already makes
direct access safe before a connection has been assigned.

Preserve the existing best-effort LOGOUT, close and reference cleanup
behavior. Exercise it in the existing Python test script and make check,
including failure injection, partial initialization and a loopback socket.
Document the fix in NEWS.adoc.

Closes: networkupstools#3619

AI assistance: OpenAI Codex with gpt-6-astra (x-high reasoning).
The human contributor remains responsible for reviewing and submitting
the change.

Signed-off-by: user01010111 <12504630+user01010111@users.noreply.github.com>
@jimklimov jimklimov added python portability We want NUT to build and run everywhere possible Connection stability issues Issues about driver<->device and/or networked connections (upsd<->upsmon...) going AWOL over time labels Sep 7, 2026
@jimklimov jimklimov added this to the 2.8.6 milestone Sep 7, 2026
@jimklimov jimklimov added the AI For good or bad, machine tools are upon us. Humans are still the responsible ones. label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

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

@AppVeyorBot

Copy link
Copy Markdown

Build nut 2.8.5.5249-master completed (commit 806444fed7 by @user01010111)

@AppVeyorBot

Copy link
Copy Markdown

@AppVeyorBot

Copy link
Copy Markdown

Build nut 2.8.5.5251-master completed (commit 5db3642959 by @jimklimov)

@jimklimov

jimklimov commented Sep 9, 2026

Copy link
Copy Markdown
Member

Regarding failed "slowbuild-run/MATRIX_TAG=gnu99-gnu++98-debian11" scenario: same issue as #3628 (comment)

UPDATE: I'm on it now

jimklimov added a commit to user01010111/nut that referenced this pull request Sep 9, 2026
…test` module on some systems [networkupstools#3627]

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

Copy link
Copy Markdown

Build nut 2.8.5.5266-master completed (commit 0d1ca0a0dc by @jimklimov)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI For good or bad, machine tools are upon us. Humans are still the responsible ones. Connection stability issues Issues about driver<->device and/or networked connections (upsd<->upsmon...) going AWOL over time portability We want NUT to build and run everywhere possible python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PyNUT disconnect() leaves the socket open on a retained client

3 participants