Skip to content

🐛 bug: Prevent app.init mutex deadlock on panic#4366

Merged
ReneWerner87 merged 3 commits into
mainfrom
copilot/fix-app-init-mutex-deadlock
May 28, 2026
Merged

🐛 bug: Prevent app.init mutex deadlock on panic#4366
ReneWerner87 merged 3 commits into
mainfrom
copilot/fix-app-init-mutex-deadlock

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 28, 2026

Description

app.init() manually released app.mutex, so a panic during service or view initialization could leave the app permanently locked. This change makes the init critical section panic-safe without changing the existing post-init hook registration order.

  • Locking fix

    • Scope the app.init() critical section inside a small inner function.
    • Use defer app.mutex.Unlock() inside that scope so panics from initServices() or Views.Load() cannot strand the mutex.
    • Keep the existing behavior where OnPostShutdown is registered only after the init lock has been released.
  • Regression coverage

    • Add a focused test that forces Views.Load() to panic during a follow-up app.init().
    • Verify a later app.Get(...) call does not block, covering the original deadlock path.
func (app *App) init() *App {
	func() {
		app.mutex.Lock()
		defer app.mutex.Unlock()

		app.initServices()
		if app.config.Views != nil {
			_ = app.config.Views.Load()
		}

		app.server = &fasthttp.Server{...}
	}()

	app.Hooks().OnPostShutdown(...)
	return app
}

Changes introduced

  • Benchmarks: No benchmark changes.
  • Documentation Update: No documentation changes.
  • Changelog/What's New: Internal reliability fix for app initialization under panic.
  • Migration Guide: No migration required.
  • API Alignment with Express: Not applicable; no API surface change.
  • API Longevity: Preserves existing behavior while hardening initialization against panic-induced deadlocks.
  • Examples: Included a minimal code snippet showing the panic-safe locking structure.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • Enhancement (improvement to existing features and functionality)
  • Documentation update (changes to documentation)
  • Performance improvement (non-breaking change which improves efficiency)
  • Code consistency (non-breaking change which improves code reliability and robustness)

Checklist

Before you submit your pull request, please make sure you meet these requirements:

  • Followed the inspiration of the Express.js framework for new functionalities, making them similar in usage.
  • Conducted a self-review of the code and provided comments for complex or critical parts.
  • Updated the documentation in the /docs/ directory for Fiber's documentation.
  • Added or updated unit tests to validate the effectiveness of the changes or new features.
  • Ensured that new and existing unit tests pass locally with the changes.
  • Verified that any new dependencies are essential and have been agreed upon by the maintainers/community.
  • Aimed for optimal performance with minimal allocations in the new code.
  • Provided benchmarks for the new code to analyze and improve upon.

Commit formatting

Please use emojis in commit messages for an easy way to identify the purpose or intention of a commit. Check out the emoji cheatsheet here: CONTRIBUTING.md

Copilot AI changed the title [WIP] Fix mutex deadlock in app.init() due to panic handling Prevent app.init mutex deadlock on panic May 28, 2026
Copilot finished work on behalf of gaby May 28, 2026 04:28
Copilot AI requested a review from gaby May 28, 2026 04:28
@ReneWerner87 ReneWerner87 added this to v3 May 28, 2026
@ReneWerner87 ReneWerner87 added this to the v3 milestone May 28, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.38%. Comparing base (0290ca8) to head (4c69a78).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4366      +/-   ##
==========================================
+ Coverage   91.33%   91.38%   +0.04%     
==========================================
  Files         132      132              
  Lines       13104    13105       +1     
==========================================
+ Hits        11969    11976       +7     
+ Misses        716      711       -5     
+ Partials      419      418       -1     
Flag Coverage Δ
unittests 91.38% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gaby gaby changed the title Prevent app.init mutex deadlock on panic 🐛 bug: Prevent app.init mutex deadlock on panic May 28, 2026
@gaby gaby marked this pull request as ready for review May 28, 2026 12:07
Copilot AI review requested due to automatic review settings May 28, 2026 12:07
@gaby gaby requested a review from a team as a code owner May 28, 2026 12:07
@gaby gaby requested review from ReneWerner87, efectn and sixcolors May 28, 2026 12:07
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Hardens App.init() against panic-induced deadlocks by making the init mutex unlock panic-safe while preserving the existing ordering where OnPostShutdown is registered only after the init lock is released.

Changes:

  • Scope the App.init() critical section in an inner function and use defer app.mutex.Unlock() to ensure the mutex is always released, even if initServices() or Views.Load() panics.
  • Keep OnPostShutdown registration outside the critical section to preserve the prior lock-release ordering.
  • Add a regression test that forces a panic during a second app.init() and verifies later route registration does not deadlock.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
app.go Makes app.init() mutex unlock panic-safe by scoping the lock and deferring Unlock() within an inner function.
app_test.go Adds a test that reproduces the prior deadlock scenario (panic during init()), asserting route registration proceeds afterward.

@ReneWerner87 ReneWerner87 merged commit f49e7d8 into main May 28, 2026
19 checks passed
@github-project-automation github-project-automation Bot moved this to Done in v3 May 28, 2026
@ReneWerner87 ReneWerner87 deleted the copilot/fix-app-init-mutex-deadlock branch May 28, 2026 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

🐛 [Bug]: app.init() holds mutex without defer — panic causes permanent deadlock

4 participants