Skip to content

[agent] Use std::_Exit() to avoid EventBase double-drive on HwAgent exit - #1525

Open
AnantKishorSharma wants to merge 1 commit into
facebook:mainfrom
AnantKishorSharma:hwagent-warmboot-exit-eventbase-race
Open

[agent] Use std::_Exit() to avoid EventBase double-drive on HwAgent exit#1525
AnantKishorSharma wants to merge 1 commit into
facebook:mainfrom
AnantKishorSharma:hwagent-warmboot-exit-eventbase-race

Conversation

@AnantKishorSharma

@AnantKishorSharma AnantKishorSharma commented Aug 18, 2026

Copy link
Copy Markdown

Summary

SplitHwAgentSignalHandler::signalReceived() performs a graceful shutdown (unregister callbacks, stop oper-delta sync, stop services, gracefulExit() which writes warm-boot state) and then calls exit(0).

exit(0) runs global/atexit destructors. During a warm-boot shutdown those destructors can re-enter and drive the still-live thrift folly::EventBase — the one hwAgentMain() is driving in server->serve() on the main thread — from the signal-handler thread. Two threads then drive the same EventBase, tripping folly's check in EventBase.cpp:

Check failed: expected == prevLoopTid
Driving an EventBase (in thread X) while it is already being driven (in thread Y) is forbidden.

The process aborts before the oper-delta ack is sent, so the peer SW agent waits out oper_delta_ack_timeout (600s) and the warm-boot iteration fails on timeout. Cold-boot exit takes the hwAgent_.reset() branch and does not hit this, which is why only warm boot is affected.

Fix

Replace exit(0) with std::_Exit(0). All required graceful cleanup (warm-boot state write via gracefulExit(), service/syncer shutdown) has already completed earlier in the handler, so the global/atexit destructors add nothing here; skipping them avoids the second EventBase drive and the resulting abort. This mirrors the existing std::_Exit(rc) usage in fboss/agent/hw/test/Main.cpp.

Test Plan

  • On hardware, forcing the HW-agent exit to bypass the atexit/global-destructor path (the behavior this change makes permanent) does not reproduce the folly EventBase "already being driven" abort on warm-boot shutdown; the agent exits cleanly and the warm-boot oper-delta ack is delivered.
  • gracefulExit() persists warm-boot state before the exit call, so std::_Exit() loses no state.
  • Cold-boot exit (the hwAgent_.reset() path) and the agent_exit_delay_s delay path are unchanged.

Runtime workaround, not a fix: agent_exit_delay_s

For anyone who can't pick up this patch yet, the same warm-boot abort can be mitigated at runtime without a rebuild via the existing agent_exit_delay_s gflag (AgentFeatures.cpp). It sleeps right before the exit(0) in this handler — after gracefulExit() has already persisted warm-boot state — giving the main thread's server->serve() loop time to unwind before the atexit/global destructors run. It can be set via the agent config's defaultCommandLineArgs (both agents read it), e.g. "agent_exit_delay_s": "5" (bump to "10" if needed).

Caveats:

  • Non-deterministic — the delay only widens the timing window; it does not remove the double-drive, so the test can still be flaky (a larger delay may help but isn't guaranteed). std::_Exit() in this PR is the deterministic fix.

@AnantKishorSharma
AnantKishorSharma requested a review from a team as a code owner August 18, 2026 10:53
@meta-cla

meta-cla Bot commented Aug 18, 2026

Copy link
Copy Markdown

Hi @AnantKishorSharma!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

@vvasavada-nexthop

Copy link
Copy Markdown

This is already being fixed by #1492. The change there makes sure destructors are executed and it also targets other exit paths (disconnects, s/w agent, etc.).

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.

3 participants