Skip to content

Use weak_ptr and wait for shutdown#7337

Merged
TheMarex merged 2 commits intomasterfrom
patrick/fix_beast_leak
Jan 22, 2026
Merged

Use weak_ptr and wait for shutdown#7337
TheMarex merged 2 commits intomasterfrom
patrick/fix_beast_leak

Conversation

@TheMarex
Copy link
Copy Markdown
Member

@TheMarex TheMarex commented Jan 21, 2026

Issue

As noted by @MarcelloPerathoner ASAN reports some leaks with the new beast code. This is cause by two things:

  1. We don't properly wait before we consider the server stopped.
  2. We put shared_ptr in the payloads of the async handler functions. Even if there is no busy connection they sit in the queue with a handle. This changes it to weak_ptr which promotes to shared_ptr if we actually start execution.

Tasklist

  • review
  • adjust for comments

Requirements / Relations

#7328

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

This pull request addresses memory leaks detected by ASAN in the Beast-based HTTP server implementation. The changes focus on proper shutdown synchronization and preventing reference cycles in asynchronous handlers.

Changes:

  • Added a wait loop in the Stop() method to ensure the io_context has fully stopped before returning
  • Changed async handler captures from shared_ptr to weak_ptr in DoAccept() to prevent reference cycles

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread include/server/server.hpp Outdated
@TheMarex TheMarex force-pushed the patrick/fix_beast_leak branch from dbc8830 to 8f485b0 Compare January 21, 2026 16:59
@TheMarex TheMarex requested a review from DennisOSRM January 22, 2026 16:31
@TheMarex TheMarex merged commit 0a2edf8 into master Jan 22, 2026
19 checks passed
@TheMarex TheMarex deleted the patrick/fix_beast_leak branch January 22, 2026 22:02
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