fix memory when close window #809
Conversation
WalkthroughAdds a new TabPresenter method to bulk-close tabs for a window, invokes it from the WindowPresenter's default-close path, and updates the shared presenter interface to expose the new method. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant WP as WindowPresenter
participant TP as TabPresenter
participant T as Tab (per-tab)
User->>WP: Request close window(windowId)
WP->>WP: Determine default close allowed
note right of WP: default close path
WP->>TP: closeTabs(windowId)
rect rgba(200,230,255,0.3)
TP->>TP: tabIds = windowTabs[windowId] || []
loop for each tabId
TP-->>T: closeTab(tabId) (dispatched, not awaited)
end
end
WP->>WP: Proceed with window close
WP-->>User: Window closed (tab closures dispatched)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/presenter/windowPresenter/index.ts (1)
793-799: Race condition: tabs may survive window close; await teardown before closing the window.
presenter.tabPresenter.closeTabs(windowId)is not awaited and the default close proceeds immediately, so the BrowserWindow can destroy before tab teardowns finish. On Windows this frequently leaves tab processes in a suspended state and leaks memory—the very issue this PR targets.Minimal in-hunk change (also make the enclosing 'close' listener async as shown below):
- presenter.tabPresenter.closeTabs(windowId) + await presenter.tabPresenter.closeTabs(windowId)Apply the following adjustment to the 'close' handler to ensure ordering (outside this hunk):
// Change listener to async and gate default close until tabs are torn down shellWindow.on('close', async (event) => { console.log(`Window ${windowId} close event. isQuitting: ${this.isQuitting}, Platform: ${process.platform}.`) if (!this.isQuitting) { const shouldQuitOnClose = this.configPresenter.getCloseToQuit() const shouldPreventDefault = windowId === this.mainWindowId && !shouldQuitOnClose if (shouldPreventDefault) { // existing hide flow... event.preventDefault() /* ... */ } else { // allow closing, but first tear down tabs deterministically event.preventDefault() try { await presenter.tabPresenter.closeTabs(windowId) } catch (e) { console.error(`Failed to close all tabs for window ${windowId}:`, e) } // Now actually close without re-entering the handler shellWindow.destroy() // or: shellWindow.removeAllListeners('close'); shellWindow.close() } } else { console.log(`Window ${windowId}: isQuitting is true, allowing default close behavior.`) } })
🧹 Nitpick comments (1)
src/shared/presenter.d.ts (1)
187-187: Document the new API contract and intended sequencing.Add a brief JSDoc to clarify that all tab teardowns are awaited (sequentially) and the call is idempotent. This avoids ambiguity for callers (e.g., window close flow).
closeTab(tabId: number): Promise<boolean> - closeTabs(windowId: number): Promise<void> + /** + * Close all tabs that belong to the given window. + * Implementations should await each tab teardown to ensure WebContents are destroyed + * before the BrowserWindow exits. Safe to call multiple times; no-op if no tabs exist. + */ + closeTabs(windowId: number): Promise<void>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/main/presenter/tabPresenter.ts(1 hunks)src/main/presenter/windowPresenter/index.ts(1 hunks)src/shared/presenter.d.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/main/presenter/windowPresenter/index.tssrc/shared/presenter.d.tssrc/main/presenter/tabPresenter.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/windowPresenter/index.tssrc/main/presenter/tabPresenter.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
From main to renderer, broadcast events via EventBus using mainWindow.webContents.send()
Files:
src/main/presenter/windowPresenter/index.tssrc/main/presenter/tabPresenter.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪（如适用）、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别Enable and adhere to strict TypeScript type checking
Files:
src/main/presenter/windowPresenter/index.tssrc/shared/presenter.d.tssrc/main/presenter/tabPresenter.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/windowPresenter/index.tssrc/main/presenter/tabPresenter.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/windowPresenter/index.tssrc/shared/presenter.d.tssrc/main/presenter/tabPresenter.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain one presenter per functional domain in src/main/presenter/
Files:
src/main/presenter/windowPresenter/index.tssrc/main/presenter/tabPresenter.ts
src/shared/*.d.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
The shared/*.d.ts files are used to define the types of objects exposed by the main process to the renderer process
Files:
src/shared/presenter.d.ts
src/shared/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
共享类型定义放在
shared目录
Files:
src/shared/presenter.d.ts
src/shared/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Put shared types and IPC contracts in src/shared/
Files:
src/shared/presenter.d.ts
🧬 Code graph analysis (1)
src/main/presenter/windowPresenter/index.ts (1)
src/main/presenter/index.ts (1)
presenter(188-188)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-check (x64)
|
关于src/main/presenter/windowPresenter/index.ts (1)的修改意见： 在'close'事件中，有event.preventDefault()阻止关闭的逻辑，不能使用异步操作。 |
2 participants
Pull Request Description
Is your feature request related to a problem? Please describe.
Memory leaks occur when closing a window in a multi-window scenario. This happens because tabs within the window are not closed when the window is closed. On Windows systems, these tab processes may be marked as energy-saving after the window becomes invisible but are not properly terminated, resulting in unreleased memory resources.
Describe the solution you'd like
Automatically close all tabs within a window when the window is closed. This ensures that all associated processes and resources are properly released, preventing memory leaks.
UI/UX changes for Desktop Application
This PR does not introduce any visible UI/UX changes. The tab closing process happens automatically in the background when a window is closed, maintaining the same user interaction patterns.
Platform Compatibility Notes
Additional context
This fix addresses a long-standing issue with memory consumption in multi-window workflows. By ensuring proper cleanup of tab processes during window closure, we maintain system performance even after extended use with multiple windows.
Pull Request Description (中文)
你的功能请求是否与某个问题有关？请描述一下。
多窗口场景下关闭窗口时会发生内存泄漏。这是因为关闭窗口时未关闭窗口内的标签页，在Windows系统中，窗口关闭后标签页进程可能被标记为节能状态但不会被正确终止，导致内存资源无法释放。
请描述你希望的解决方案
关闭窗口时自动关闭其内部所有标签页，确保所有相关进程和资源被正确释放，防止内存泄漏。
桌面应用程序的 UI/UX 更改
此PR不会引入任何可见的UI/UX变化。标签页关闭过程在窗口关闭时在后台自动进行，保持原有的用户交互模式。
平台兼容性注意事项
附加背景
此修复解决了多窗口工作流程中长期存在的内存消耗问题。通过确保窗口关闭时正确清理标签页进程，即使在长时间使用多窗口的情况下，也能保持系统性能。
Summary by CodeRabbit
New Features
Improvements