fix: handle race condition when logging to removed logger (fixes #318224)#318229
Merged
vs-code-engineering[bot] merged 1 commit intoMay 25, 2026
Merged
Conversation
) The memory leak fix in 1fc96f7 added cleanup of loggers on deregistration. This created a race condition where IPC log messages from renderer processes arrive after the logger has been removed from the map, causing an unhandled error. Replace the throw with a silent return since log messages for a removed logger are stale and should be discarded. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dmitrivMS
approved these changes
May 25, 2026
roblourens
approved these changes
May 25, 2026
dileepyavan
pushed a commit
that referenced
this pull request
May 27, 2026
) (#318229) The memory leak fix in 1fc96f7 added cleanup of loggers on deregistration. This created a race condition where IPC log messages from renderer processes arrive after the logger has been removed from the map, causing an unhandled error. Replace the throw with a silent return since log messages for a removed logger are stale and should be discarded. Co-authored-by: vs-code-engineering[bot] <122617954+vs-code-engineering[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The error
Create the logger before loggingis thrown inLoggerChannel.log()when a renderer process sends an IPC log message for a logger that has already been removed from theloggersmap. This is a race condition introduced by commit1fc96f74which added eager logger cleanup on deregistration — IPC messages already in the pipeline arrive after the logger entry is deleted.Hits: ~113k across 1600+ users on 1.122.0-insider builds since 2026-05-22.
Fixes #318224
Recommended reviewer:
@SimonSiefkeCulprit Commit
1fc96f74— "fix: memory leak in LoggerChannel (#315875)" by@SimonSiefke(2026-05-21)This commit added an
onDidChangeLoggerslistener that deletes loggers from theResourceMapwhen they are deregistered. The error first appeared the next day (2026-05-22), confirming causality.Code Flow
sequenceDiagram participant Renderer participant IPC as IPC Pipeline participant LC as LoggerChannel participant LS as LoggerMainService Renderer->>IPC: log(file, messages) LS->>LC: onDidChangeLoggers({removed: [file]}) LC->>LC: loggers.delete(file) IPC->>LC: call('log', [file, messages]) LC->>LC: loggers.get(file) → undefined LC->>LC: throw Error('Create the logger before logging')Affected Files
src/vs/platform/log/electron-main/logIpc.tslog()method throws when logger not foundRepro Steps
LoggerChannel.log()throws because the logger was already removed from the mapThis is a timing-dependent race that occurs on all platforms (Windows, Mac, Linux).
How the Fix Works
Chosen approach (
src/vs/platform/log/electron-main/logIpc.ts:74):Replaced the
throw new Error('Create the logger before logging')with a guard clause that returns early when the logger is not found. This is the correct fix because the error is produced at this exact site — it is the error construction code, not a downstream crash site. The invalid state (missing logger) is produced by theonDidChangeLoggerscleanup handler in the same class (line 20-23) racing with in-flight IPC messages. Since we cannot prevent the renderer from sending messages that are already in the IPC pipeline, the appropriate fix is to gracefully discard stale log messages rather than throw.After this change,
logIpc.ts:74cannot produce the error because whenloggers.get(file)returnsundefined, execution returns immediately instead of reaching the throw statement.Alternatives considered:
Recommended Owner
@SimonSiefke— authored the culprit commit1fc96f74that introduced the race condition.