Skip to content

fix: run preClose hook exactly once per declaring instance - #6940

Merged
metcoder95 merged 1 commit into
fastify:mainfrom
contactjawad:fix-preclose-hook-once
Aug 14, 2026
Merged

metcoder95 merged 1 commit into
fastify:mainfrom
contactjawad:fix-preclose-hook-once

Conversation

@contactjawad

Copy link
Copy Markdown
Contributor

What

A single preClose hook added to a Fastify instance is executed once for every instance in the encapsulation subtree (the declaring instance plus each descendant plugin) instead of exactly once.

Reproduction

const fastify = Fastify()
let count = 0

fastify.register(async (child) => {
  child.get('/x', async () => 'ok')
})
fastify.addHook('preClose', async () => { count++ })

await fastify.ready()
await fastify.close()

console.log(count) // 2, expected 1

With two levels of nested plugins the same hook runs 3 times. An equivalent onReady hook in the identical setup correctly runs once.

Root cause

In addHook, the hooks onReady, onListen, and onRoute are special-cased to be stored only on the declaring instance via this[kHooks].add. preClose was omitted from that list, so it falls into the else branch and goes through _addHook, which copies the hook into every child via this[kChildren].forEach. At close time, hookRunnerApplication('preClose') already walks the entire child tree, so each propagated copy fires again.

That preClose is not meant to be inherited is already established elsewhere in the codebase: buildHooks in lib/hooks.js resets onReady, onListen, and preClose to empty arrays for children. preClose was simply left off the special-case list in addHook.

Fix

Add preClose to the same special-case branch as onReady/onListen/onRoute so it is stored only on the declaring instance and not propagated into children. Since hookRunnerApplication already recurses the child tree and buildHooks already excludes preClose from inheritance, each declared hook now fires exactly once.

Tests

Added two regression tests in test/close.test.js (a single child plugin and nested child plugins) asserting the hook runs exactly once. Both fail on main (count 2 and 3) and pass with this change. The full test/close.test.js suite (26 tests) stays green.

Checklist

  • run npm run test and the linter
  • tests are included
  • commit is signed off (DCO)

A single preClose hook added to an instance was executed once for every
instance in the encapsulation subtree (the declaring instance plus each
descendant plugin) instead of exactly once. In addHook, onReady/onListen/
onRoute are special-cased to store the hook only on the declaring instance,
but preClose was omitted, so it fell into the propagating _addHook path
(which copies the hook into every child). Since hookRunnerApplication('preClose')
already recurses the child tree at close time, each declared hook re-fired
once per descendant.

Add preClose to the same special-case branch. buildHooks already resets
children's preClose to [], so preClose was never meant to be inherited; this
makes each declared hook fire exactly once.

Signed-off-by: contactjawad <[email protected]>

@jean-michelet jean-michelet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good catch

@jean-michelet
jean-michelet requested a review from a team August 13, 2026 08:40
@Eomm Eomm added bugfix Issue or PR that should land as semver patch backport 5.x Issue or pr that should be backported to Fastify v5 labels Aug 13, 2026
@metcoder95
metcoder95 merged commit f5ef344 into fastify:main Aug 14, 2026
36 checks passed
climba03003 pushed a commit that referenced this pull request Aug 14, 2026
…6954)

(cherry picked from commit f5ef344)

Signed-off-by: contactjawad <[email protected]>
Co-authored-by: Jawad Ali <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 5.x Issue or pr that should be backported to Fastify v5 bugfix Issue or PR that should land as semver patch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants