Skip to content

common: Changed to leak logger singleton to prevent hanging on Windows#22273

Open
rillomas wants to merge 4 commits intoggml-org:masterfrom
rillomas:fix-hang-on-windows
Open

common: Changed to leak logger singleton to prevent hanging on Windows#22273
rillomas wants to merge 4 commits intoggml-org:masterfrom
rillomas:fix-hang-on-windows

Conversation

@rillomas
Copy link
Copy Markdown
Contributor

@rillomas rillomas commented Apr 23, 2026

Overview

Added workaround for #22142.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: YES (Copilot GPT5.4 medium). Used for issue analysis and solution recommendation.

@rillomas rillomas requested a review from a team as a code owner April 23, 2026 06:03
@rillomas rillomas mentioned this pull request Apr 23, 2026
Open
@ggerganov ggerganov added the merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. label Apr 23, 2026
@ggerganov ggerganov removed the merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. label Apr 24, 2026
@ggerganov
Copy link
Copy Markdown
Member

ggerganov commented Apr 24, 2026

Does not seem to work - the tests are failing

@rillomas
Copy link
Copy Markdown
Contributor Author

rillomas commented Apr 24, 2026

Let me have a look again.

@rillomas rillomas marked this pull request as draft April 24, 2026 06:32
@rysweet rysweet mentioned this pull request Apr 24, 2026
Open
5 tasks
@rillomas
Stopped using static vector
3d11a9b 
Using std::vector will cause g_col to be released before the logger thread exits, causing the logger thread to touch freed memory causing a crash
@rillomas rillomas marked this pull request as ready for review April 24, 2026 08:27
@rillomas
Copy link
Copy Markdown
Contributor Author

rillomas commented Apr 24, 2026 •
edited
Loading

I've fixed the crashing on Linux. It seems like g_col being a vector was causing the heap to get freed at teardown phase while the logger thread is running, causing the crash. Previously the common_log destructor was (presumably) joining the logger thread first assuring g_col was never touched after release.

@rillomas rillomas requested a review from ggerganov as a code owner April 24, 2026 08:37
@github-actions github-actions Bot added the testing Everything test related label Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pwilkin pwilkin pwilkin approved these changes

@ggerganov ggerganov Awaiting requested review from ggerganov ggerganov is a code owner

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

testing Everything test related

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rillomas @ggerganov @pwilkin