Skip to content

Bound the fqdn lookup in the hostname resolvers - #177

Open
silug wants to merge 1 commit into
OpenVoxProject:mainfrom
silug:hostname-lookup-timeout
Open

Bound the fqdn lookup in the hostname resolvers#177
silug wants to merge 1 commit into
OpenVoxProject:mainfrom
silug:hostname-lookup-timeout

Conversation

@silug

@silug silug commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Short description

When the short hostname carries no domain, the hostname resolvers ask the name service for the canonical name with getaddrinfo(3). That call had no upper bound, so a hung resolver stalled fact collection until the agent's runtimeout fired an hour later (OpenVoxProject/openvox#485).

Switch to Addrinfo.getaddrinfo and pass a 10 second timeout on Rubies that honour it (MRI 4.0 and later; older MRI accepts and ignores the keyword, JRuby does not accept it, so it is omitted there). On Linux, a timed-out lookup no longer falls through to the FFI getaddrinfo fallback, which has no timeout of its own and holds the GVL while it waits. The domain then comes from /etc/resolv.conf as it already does when the lookup fails.

Generated by Claude Code

Checklist

I have:

When the short hostname carries no domain, the hostname resolvers ask the
name service for the canonical name with getaddrinfo(3). That call had no
upper bound, so a hung resolver stalled fact collection until the agent's
runtimeout fired an hour later (OpenVoxProject/openvox#485).

Switch to Addrinfo.getaddrinfo and pass a 10 second timeout on Rubies that
honour it (MRI 4.0 and later; older MRI accepts and ignores the keyword,
JRuby does not accept it, so it is omitted there). On Linux, a timed-out
lookup no longer falls through to the FFI getaddrinfo fallback, which has
no timeout of its own and holds the GVL while it waits. The domain then
comes from /etc/resolv.conf as it already does when the lookup fails.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Steven Pritchard <steven.pritchard@gmail.com>
@silug

silug commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

This comment was generated by Claude Code, which wrote the change on this branch; posted by @silug.

Why this call, and why a timeout works here

The debug log in OpenVoxProject/openvox#485 (comment of Sep 7) shows fact collection stopping right after Only managed to read hostname: annalee, no domain was found. and nothing further for 58 minutes until the agent's runtimeout fired. The next statement in Facter::Resolvers::Linux::Hostname after that log line is the getaddrinfo lookup for the canonical name, so that lookup is where the run was stuck. The run timeout fired at exactly 3600 s, which means the GVL was free at that moment; that is consistent with Ruby's threaded getaddrinfo (Ruby 3.3+), whose caller waits without the GVL, and rules out the FFI fallback for that phase, since the FFI call holds the GVL.

What timeout: does on each Ruby

Checked against the Ruby source tree:

  • Addrinfo.getaddrinfo has accepted the timeout: keyword since 3.0 (rb_get_kwargs with id_timeout), but on 3.0 through 3.4 call_getaddrinfo drops it before calling rsock_getaddrinfo, which has no timeout parameter. The keyword is a no-op there.
  • In 4.0.0 rsock_getaddrinfo gained a fifth timeout argument and the helper-thread wait uses a timed condvar wait, so the keyword is honoured.
  • Socket.getaddrinfo (the method previously used here) never accepts the keyword on any version.
  • JRuby's Addrinfo.getaddrinfo does not accept the keyword, which is why the branch only passes it when RUBY_ENGINE == 'ruby' and the version is 4.0 or later.

Test results

  • spec/facter/resolvers/hostname_spec.rb and spec/facter/resolvers/linux/hostname_spec.rb: 77 examples, 0 failures on Ruby 4.0.6.

  • rubocop on the four changed files: no offenses.

  • End to end on Ruby 4.0.6 in a network namespace with a dummy interface, /etc/hosts emptied, nsswitch.conf set to hosts: files dns, and resolv.conf pointing at an address that never answers with options timeout:30 attempts:5 (glibc alone would wait several minutes):

    gethostname=ai-dev
    fqdn="ai-dev.example.test" domain="example.test" after 10.0s
    

    The lookup returned at the 10 s bound, the FFI fallback was not called, and the domain came from the search line in resolv.conf.

  • The same setup with the original code, using a bare Addrinfo.getaddrinfo(..., timeout: 1) to confirm the mechanism: IO::TimeoutError: user specified timeout for stalled.example. port 80 after 1.0s.

Behaviour notes for review

  • A timed-out lookup returns nil for the canonical name and skips retrieve_fqdn_for_host_with_ffi. Any other failure (for example EAI_NONAME) still falls through to the FFI fallback as before.
  • The FFI getaddrinfo in Facter::Util::Resolvers::Ffi::Hostname is still attached without blocking: true, so it holds the GVL for the duration of the call. This branch avoids reaching it after a timeout but does not change the call itself.
  • The 10 s constant is duplicated in the two resolver classes, matching the existing duplication between them.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is narrowly scoped, covered by updated unit tests, and only minor comment typos were identified.

Pull request overview

This pull request bounds FQDN lookups in the hostname resolvers to prevent fact collection from stalling when DNS/name service resolution hangs, by switching to Addrinfo.getaddrinfo and applying a 10-second timeout on Ruby versions that honor it.

Changes:

  • Use Addrinfo.getaddrinfo (instead of Socket.getaddrinfo) for canonical-name lookups and introduce FQDN_LOOKUP_TIMEOUT = 10.
  • Conditionally pass timeout: only on MRI Ruby >= 4.0, omitting it where it’s ignored or unsupported.
  • Add/adjust unit tests to validate timeout behavior and avoid falling back to the FFI lookup on timeout (Linux).
File summaries
File Description
lib/facter/resolvers/linux/hostname.rb Adds a bounded Addrinfo-based FQDN lookup and avoids FFI fallback on timeout.
lib/facter/resolvers/hostname.rb Adds a bounded Addrinfo-based FQDN lookup in the generic hostname resolver.
spec/facter/resolvers/linux/hostname_spec.rb Updates tests to stub Addrinfo.getaddrinfo and adds timeout-related coverage.
spec/facter/resolvers/hostname_spec.rb Updates tests for Addrinfo.getaddrinfo and adds coverage for timeout/no-timeout behavior.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/facter/resolvers/hostname.rb
Comment thread lib/facter/resolvers/linux/hostname.rb
@bastelfreak

Copy link
Copy Markdown
Contributor

This looks okay but I'm not 100% confident in merging. Anybody else around who wants to take a look? Or maybe create a test build?

@miharp

miharp commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

I did a test build: swapped the two resolver files from this PR into a 9.0.0-rc1 agent (Ruby 4.0.6, CentOS Stream 9) in my lab, with a short hostname and the resolver black-holed so glibc gives up after 40 s per lookup.

  • facter networking.fqdn: 81 s -> 11 s. The 10 s timeout fires (IO::TimeoutError), the FFI fallback is skipped, and the returned value stays agent01.
  • puppet agent -t: the fqdn lookup inside fact collection drops from 80 s to 10 s and the run completes with a faster fact collection. Two ~40 s delays elsewhere in the run remained in both the patched and unpatched runs; they are unrelated to openfact (see below).
  • Under these lab conditions the PR also reproduces the fork-lock mechanism investigated in openvox#485: the timed-out lookup thread keeps running in glibc and holds Ruby's fork lock, so the first Kernel#fork after facts blocked for the remaining 30 s with the GVL held (a Timeout around it only fired after the fork returned). It can also occur after server connections under these lab conditions, independently of openfact, so the fork lock needs handling in openvox or Ruby either way. This PR is still worth having for the fact-collection part.

One thing to settle before merging: on JRuby, Addrinfo#canonname performs Java's reverse-name lookup at call time, while the old Socket.getaddrinfo returned the numeric address in that slot under the default reverse-lookup setting, so JRuby can use a reverse-resolved name instead of falling through to FFI on Linux, and gains another blocking lookup. Keeping Socket.getaddrinfo on JRuby would avoid that. Small comment nit: JRuby does accept timeout:, it parses and ignores it.

Scripts and logs available on request. +1 from me once the JRuby point is addressed.

@bastelfreak

Copy link
Copy Markdown
Contributor

@miharp thanks a lot for testing!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants