fix: patch all P1 bugs from project review
- pubsub: add sender.isDestroyed() checks + destroyed event auto-cleanup (P1#10)
- pubsub: add .catch() to monitor() promise + isDestroyed check (P1#11)
- connection: move server.on('error') before server.listen() (P1#12)
- preload: onSubscribeMessage/onMonitorMessage return cleanup functions (P1#9)
- CliView: save and call IPC listener cleanup in onUnmounted (P1#9)
- KeyDetail: move setInterval into onMounted, cleanup in onUnmounted (P1#8)
- key store: add loadKeyVersion guard to prevent race condition (P1#13)
- ListEditor: LSET sentinel + LREM for index-based deletion (P1#14)
- ipc-handlers: remove non-null assertion, add null guard in reorderConnections (P1#16)
- StreamEditor: add Confirm/Cancel buttons to Trim dialog (P1#17)
- KeyBrowser: remove { immediate: true } to prevent double scan (P1#18)
- P1#7 MULTI/EXEC: investigated, not a bug (Redis server handles queuing)
- docs: update PROJECT_REVIEW.md with P1 fix status
This commit is contained in:
+28
-17
@@ -4,7 +4,7 @@
|
||||
> 审查方式:并行扫描主进程、渲染进程、IPC 契约一致性
|
||||
> 审查范围:主进程 10 文件、渲染进程 ~40 文件、IPC 65 通道
|
||||
> 发现问题:54 个(P0×6, P1×12, P2×17, P3×19)
|
||||
> 已修复:**P0×6 全部修复** (2026-07-12)
|
||||
> 已修复:**P0×6 + P1×11 全部修复** (2026-07-12)
|
||||
> 可开发需求:26 项(高优先级 6,中 11,低 9)
|
||||
|
||||
---
|
||||
@@ -32,22 +32,22 @@
|
||||
| 5 | `src/main/redis/connection.ts` (3处) | **超时竞态**:Sentinel/Cluster/普通模式的 setTimeout 均保存 timer ID,在 ready/error 路径调用 clearTimeout | **已修复** |
|
||||
| 6 | `src/main/credential.ts:4-5` | **safeStorage 不可用时回退明文**:encrypt/decrypt 在 safeStorage 不可用时抛出错误,decrypt catch 保留 base64 兼容旧数据并加 console.warn | **已修复** |
|
||||
|
||||
### P1 - 严重 Bug(功能错误)
|
||||
### P1 - 严重 Bug(功能错误) - 全部已处理
|
||||
|
||||
| # | 位置 | 问题 |
|
||||
|---|------|------|
|
||||
| 7 | `CliView.vue:52-101` | **MULTI/EXEC 事务逻辑错误**:事务模式下每条命令仍通过 IPC 单独发送到 Redis,服务器立即执行而非排队。完全破坏事务语义。应本地排队,EXEC 时 pipeline 一次性发送。 |
|
||||
| 8 | `KeyDetail.vue:43-68` | **TTL 定时器模块级泄漏**:`setInterval` 在 `<script setup>` 顶层立即创建,非 `onMounted`。组件销毁后定时器仍每秒运行访问已卸载响应式数据。 |
|
||||
| 9 | `CliView.vue:380-391` | **IPC 监听器未清理**:`onSubscribeMessage`/`onMonitorMessage` 在 `onUnmounted` 中未移除。每次切换标签页累积监听器。 |
|
||||
| 10 | `src/main/redis/pubsub.ts:13,24-31` | **PubSub sender 失效**:存储 `WebContents` 引用,窗口重载(HMR/导航)后失效,消息静默丢弃。无检测/重建机制。 |
|
||||
| 11 | `src/main/redis/pubsub.ts:57` | **Monitor 未处理 rejection**:`monitor.monitor().then(...)` 无 `.catch()`,失败时未处理 Promise 拒绝。 |
|
||||
| 12 | `src/main/redis/connection.ts:96-104` | **SSH error 监听器注册过晚**:`server.listen()` 在 `server.on('error')` 之前,期间错误未捕获。 |
|
||||
| 13 | `key.ts:135-203` | **loadKeyData 竞态**:快速点击不同 key 时后 resolve 的响应覆盖最新选中 key 的数据。 |
|
||||
| 14 | `ListEditor.vue:84-93` | **LREM 按值删除删错项**:重复值时删除第一个匹配,非当前选中索引。 |
|
||||
| 15 | `connection.ts:80-84`(store) | **saveConnection 突变传入对象**:直接修改 `conn.auth` 为加密串,若调用方传响应式对象则 UI 显示加密密码。 | **已修复**(P0#1 修复时创建副本而非修改原对象) |
|
||||
| 16 | `ipc-handlers.ts:71` | **reorderConnections 崩溃**:非空断言 `!`,ID 不匹配时 `TypeError`。 |
|
||||
| 17 | `StreamEditor.vue:357-361` | **Trim 对话框无确认按钮**:打开后只能关闭,无法执行 trim。 |
|
||||
| 18 | `KeyBrowser.vue` + `DbSelector.vue` | **双重扫描**:KeyBrowser 的 `watch immediate` 与 DbSelector 的 `onMounted` 都调 `scanKeys`,首次连接扫两次。 |
|
||||
| # | 位置 | 问题 | 状态 |
|
||||
|---|------|------|------|
|
||||
| 7 | `CliView.vue:52-101` | **MULTI/EXEC 事务逻辑错误**(误报:Redis 服务器在 MULTI 后自动排队命令,当前实现通过同一连接发送,功能正确) | **非 bug** |
|
||||
| 8 | `KeyDetail.vue:43-68` | **TTL 定时器模块级泄漏**:`setInterval` 移入 `onMounted`,`onUnmounted` 中清理 | **已修复** |
|
||||
| 9 | `CliView.vue:380-391` | **IPC 监听器未清理**:preload 的 `onSubscribeMessage`/`onMonitorMessage` 返回清理函数,CliView 在 `onUnmounted` 中调用 | **已修复** |
|
||||
| 10 | `src/main/redis/pubsub.ts` | **PubSub sender 失效**:添加 `sender.isDestroyed()` 检查 + `sender.once('destroyed', ...)` 自动清理 | **已修复** |
|
||||
| 11 | `src/main/redis/pubsub.ts:57` | **Monitor 未处理 rejection**:添加 `.catch()` 处理 Promise 拒绝 + `isDestroyed` 检查 | **已修复** |
|
||||
| 12 | `src/main/redis/connection.ts:96-104` | **SSH error 监听器注册过晚**:`server.on('error')` 移到 `server.listen()` 之前 | **已修复** |
|
||||
| 13 | `key.ts:135-203` | **loadKeyData 竞态**:引入 `loadKeyVersion` 版本号守卫,过时响应被丢弃 | **已修复** |
|
||||
| 14 | `ListEditor.vue:84-93` | **LREM 按值删除删错项**:改为 LSET 哨兵值 + LREM 按索引删除 | **已修复** |
|
||||
| 15 | `connection.ts:80-84`(store) | **saveConnection 突变传入对象**:P0#1 修复时创建副本而非修改原对象 | **已修复** |
|
||||
| 16 | `ipc-handlers.ts:71` | **reorderConnections 崩溃**:移除非空断言,添加 null 守卫 + filter | **已修复** |
|
||||
| 17 | `StreamEditor.vue:357-361` | **Trim 对话框无确认按钮**:添加 Cancel/Confirm 按钮调用 `trimStream()` | **已修复** |
|
||||
| 18 | `KeyBrowser.vue` + `DbSelector.vue` | **双重扫描**:移除 KeyBrowser watch 的 `{ immediate: true }` | **已修复** |
|
||||
|
||||
### P2 - 应修复(质量/性能)
|
||||
|
||||
@@ -191,7 +191,7 @@
|
||||
|
||||
- **审查范围**:主进程 10 文件、渲染进程 ~40 文件、IPC 65 通道
|
||||
- **发现问题**:54 个(P0×6, P1×12, P2×17, P3×19)
|
||||
- **已修复**:P0×6 + P1×1 = **7 个** (2026-07-12)
|
||||
- **已修复**:P0×6 + P1×11 = **17 个** (2026-07-12)
|
||||
- **安全问题**:6 个(P0 全部已修复)
|
||||
- **死代码**:6 处
|
||||
- **可开发需求**:26 项(高优先级 6,中 11,低 9)
|
||||
@@ -207,3 +207,14 @@
|
||||
| P0#5 超时竞态 | 3处 setTimeout 保存 timer ID,ready/error 路径 clearTimeout | `connection.ts` |
|
||||
| P0#6 safeStorage 回退 | encrypt/decrypt 在 safeStorage 不可用时抛出错误 | `credential.ts` |
|
||||
| P1#15 突变副作用 | saveConnection 创建副本而非修改原对象(随 P0#1 修复) | `ipc-handlers.ts` |
|
||||
| P1#8 TTL 定时器泄漏 | setInterval 移入 onMounted,onUnmounted 中清理 | `KeyDetail.vue` |
|
||||
| P1#9 IPC 监听器泄漏 | preload onSubscribeMessage/onMonitorMessage 返回清理函数,CliView onUnmounted 调用 | `preload/index.ts`, `preload/index.d.ts`, `CliView.vue` |
|
||||
| P1#10 PubSub sender 失效 | 添加 sender.isDestroyed() 检查 + destroyed 事件自动清理 | `pubsub.ts` |
|
||||
| P1#11 Monitor 未处理 rejection | 添加 .catch() + isDestroyed 检查 | `pubsub.ts` |
|
||||
| P1#12 SSH error 监听器过晚 | server.on('error') 移到 server.listen() 之前 | `connection.ts` |
|
||||
| P1#13 loadKeyData 竞态 | 引入 loadKeyVersion 版本号守卫,丢弃过时响应 | `stores/key.ts` |
|
||||
| P1#14 LREM 删错项 | 改为 LSET 哨兵值 + LREM 按索引删除 | `ListEditor.vue` |
|
||||
| P1#16 reorderConnections 崩溃 | 移除非空断言,添加 null 守卫 + filter | `ipc-handlers.ts` |
|
||||
| P1#17 Trim 对话框无确认 | 添加 Cancel/Confirm 按钮调用 trimStream() | `StreamEditor.vue` |
|
||||
| P1#18 双重扫描 | 移除 KeyBrowser watch 的 { immediate: true } | `KeyBrowser.vue` |
|
||||
| P1#7 MULTI/EXEC(误报) | 调查确认:Redis 服务器在 MULTI 后自动排队,当前实现正确 | 无需修改 |
|
||||
|
||||
@@ -95,10 +95,11 @@ export function registerIpcHandlers(): void {
|
||||
ipcMain.handle('storage:reorderConnections', (_e, ids: string[]) => {
|
||||
const connections = store.get('connections', [])
|
||||
const reordered = ids.map((id, i) => {
|
||||
const conn = connections.find((c: ConnectionConfig) => c.id === id)!
|
||||
const conn = connections.find((c: ConnectionConfig) => c.id === id)
|
||||
if (!conn) return null
|
||||
conn.order = i
|
||||
return conn
|
||||
})
|
||||
}).filter((c): c is ConnectionConfig => c !== null)
|
||||
store.set('connections', reordered)
|
||||
return reordered
|
||||
})
|
||||
|
||||
@@ -93,15 +93,15 @@ async function createSshTunnel(opts: ConnectionOptions): Promise<{ localPort: nu
|
||||
)
|
||||
})
|
||||
|
||||
server.listen(0, '127.0.0.1', () => {
|
||||
const localPort = (server.address() as AddressInfo).port
|
||||
resolve({ localPort, sshClient, server })
|
||||
})
|
||||
|
||||
server.on('error', (err) => {
|
||||
sshClient.end()
|
||||
reject(new Error(`SSH tunnel server error: ${err.message}`))
|
||||
})
|
||||
|
||||
server.listen(0, '127.0.0.1', () => {
|
||||
const localPort = (server.address() as AddressInfo).port
|
||||
resolve({ localPort, sshClient, server })
|
||||
})
|
||||
})
|
||||
|
||||
sshClient.on('error', (err: Error) => {
|
||||
|
||||
@@ -19,20 +19,22 @@ export function startSubscribe(
|
||||
const sub = new Redis(existing.options)
|
||||
subClients.set(id, sub)
|
||||
|
||||
sender.once('destroyed', () => stopSubscribe(id))
|
||||
|
||||
if (isPattern) {
|
||||
sub.psubscribe(...channels)
|
||||
sub.on('pmessage', (_pattern, channel, message) => {
|
||||
sender.send('redis:subscribeMessage', { channel, message, pattern: _pattern })
|
||||
if (!sender.isDestroyed()) sender.send('redis:subscribeMessage', { channel, message, pattern: _pattern })
|
||||
})
|
||||
} else {
|
||||
sub.subscribe(...channels)
|
||||
sub.on('message', (channel, message) => {
|
||||
sender.send('redis:subscribeMessage', { channel, message })
|
||||
if (!sender.isDestroyed()) sender.send('redis:subscribeMessage', { channel, message })
|
||||
})
|
||||
}
|
||||
|
||||
sub.on('error', (err) => {
|
||||
sender.send('redis:subscribeMessage', { error: err.message })
|
||||
if (!sender.isDestroyed()) sender.send('redis:subscribeMessage', { error: err.message })
|
||||
})
|
||||
}
|
||||
|
||||
@@ -54,13 +56,17 @@ export function startMonitor(id: string, sender: WebContents): void {
|
||||
const monitor = new Redis(existing.options)
|
||||
monitorClients.set(id, monitor)
|
||||
|
||||
sender.once('destroyed', () => stopMonitor(id))
|
||||
|
||||
monitor.monitor().then((mon) => {
|
||||
mon.on('monitor', (time: string, args: string[], source: string, database: number) => {
|
||||
sender.send('redis:monitorMessage', { time, args, source, database })
|
||||
if (!sender.isDestroyed()) sender.send('redis:monitorMessage', { time, args, source, database })
|
||||
})
|
||||
mon.on('error', (err: Error) => {
|
||||
sender.send('redis:monitorMessage', { error: err.message })
|
||||
if (!sender.isDestroyed()) sender.send('redis:monitorMessage', { error: err.message })
|
||||
})
|
||||
}).catch(err => {
|
||||
if (!sender.isDestroyed()) sender.send('redis:monitorMessage', { error: err.message })
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
Vendored
+2
-2
@@ -125,11 +125,11 @@ export interface ElectronAPI {
|
||||
// Pub/Sub
|
||||
subscribe: (id: string, channels: string[], isPattern: boolean) => Promise<any>
|
||||
unsubscribe: (id: string) => Promise<void>
|
||||
onSubscribeMessage: (callback: (msg: any) => void) => void
|
||||
onSubscribeMessage: (callback: (msg: any) => void) => () => void
|
||||
// Monitor
|
||||
monitor: (id: string) => Promise<any>
|
||||
monitorStop: (id: string) => Promise<void>
|
||||
onMonitorMessage: (callback: (msg: any) => void) => void
|
||||
onMonitorMessage: (callback: (msg: any) => void) => () => void
|
||||
}
|
||||
updater: {
|
||||
check: () => Promise<any>
|
||||
|
||||
@@ -151,7 +151,9 @@ const electronAPI = {
|
||||
unsubscribe: (id: string): Promise<void> =>
|
||||
ipcRenderer.invoke('redis:unsubscribe', id),
|
||||
onSubscribeMessage: (callback: (msg: any) => void) => {
|
||||
ipcRenderer.on('redis:subscribeMessage', (_e, msg) => callback(msg))
|
||||
const handler = (_e: any, msg: any) => callback(msg)
|
||||
ipcRenderer.on('redis:subscribeMessage', handler)
|
||||
return () => ipcRenderer.removeListener('redis:subscribeMessage', handler)
|
||||
},
|
||||
// Monitor
|
||||
monitor: (id: string): Promise<any> =>
|
||||
@@ -159,7 +161,9 @@ const electronAPI = {
|
||||
monitorStop: (id: string): Promise<void> =>
|
||||
ipcRenderer.invoke('redis:monitorStop', id),
|
||||
onMonitorMessage: (callback: (msg: any) => void) => {
|
||||
ipcRenderer.on('redis:monitorMessage', (_e, msg) => callback(msg))
|
||||
const handler = (_e: any, msg: any) => callback(msg)
|
||||
ipcRenderer.on('redis:monitorMessage', handler)
|
||||
return () => ipcRenderer.removeListener('redis:monitorMessage', handler)
|
||||
},
|
||||
},
|
||||
updater: {
|
||||
|
||||
@@ -81,11 +81,14 @@ async function saveEdit() {
|
||||
}
|
||||
}
|
||||
|
||||
async function removeItem(item: string) {
|
||||
async function removeItem(index: number) {
|
||||
if (!connStore.activeId || !keyStore.selectedKey) return
|
||||
try {
|
||||
await ElMessageBox.confirm(t('editor.removeElementConfirm'), t('common.confirm'))
|
||||
await window.electronAPI.redis.listRemove(connStore.activeId, keyStore.selectedKey, 1, item)
|
||||
// Use LSET + LREM sentinel pattern to delete by index instead of by value
|
||||
const sentinel = `\x00__JREDM_DEL_${Date.now()}_${Math.random()}__`
|
||||
await window.electronAPI.redis.listSet(connStore.activeId, keyStore.selectedKey, pageStart + index, sentinel)
|
||||
await window.electronAPI.redis.listRemove(connStore.activeId, keyStore.selectedKey, 1, sentinel)
|
||||
await loadList()
|
||||
ElMessage.success(t('editor.removed'))
|
||||
} catch (err: any) {
|
||||
@@ -127,7 +130,7 @@ function nextPage() {
|
||||
<span class="item-value">{{ item }}</span>
|
||||
<div class="item-actions">
|
||||
<el-button size="small" text @click="startEdit(index)">{{ t('editor.edit') }}</el-button>
|
||||
<el-button size="small" type="danger" text @click="removeItem(item)">{{ t('common.delete') }}</el-button>
|
||||
<el-button size="small" type="danger" text @click="removeItem(index)">{{ t('common.delete') }}</el-button>
|
||||
</div>
|
||||
</template>
|
||||
</div>
|
||||
|
||||
@@ -358,6 +358,10 @@ async function trimStream() {
|
||||
<el-form-item :label="t('editor.maxLength')">
|
||||
<el-input-number v-model="trimMaxLen" :min="1" />
|
||||
</el-form-item>
|
||||
<template #footer>
|
||||
<el-button @click="trimming = false">{{ t('common.cancel') }}</el-button>
|
||||
<el-button type="primary" @click="trimStream()">{{ t('common.confirm') }}</el-button>
|
||||
</template>
|
||||
</el-dialog>
|
||||
</div>
|
||||
</template>
|
||||
|
||||
@@ -16,8 +16,7 @@ watch(
|
||||
keyStore.keyData = null
|
||||
await keyStore.scanKeys(newId, '0', true)
|
||||
}
|
||||
},
|
||||
{ immediate: true }
|
||||
}
|
||||
)
|
||||
</script>
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
<script setup lang="ts">
|
||||
import { computed, ref, watch, onUnmounted } from 'vue'
|
||||
import { computed, ref, watch, onMounted, onUnmounted } from 'vue'
|
||||
import { useKeyStore } from '@renderer/stores/key'
|
||||
import { useConnectionStore } from '@renderer/stores/connection'
|
||||
import { useI18n } from '@renderer/i18n'
|
||||
@@ -40,7 +40,10 @@ watch(() => keyStore.selectedTTL, (v) => {
|
||||
liveTTL.value = v
|
||||
})
|
||||
|
||||
let ttlTimer: ReturnType<typeof setInterval> | null = setInterval(async () => {
|
||||
let ttlTimer: ReturnType<typeof setInterval> | null = null
|
||||
|
||||
onMounted(() => {
|
||||
ttlTimer = setInterval(async () => {
|
||||
if (liveTTL.value > 0) {
|
||||
liveTTL.value--
|
||||
if (liveTTL.value === 0) {
|
||||
@@ -61,10 +64,14 @@ let ttlTimer: ReturnType<typeof setInterval> | null = setInterval(async () => {
|
||||
}
|
||||
}
|
||||
}
|
||||
}, 1000)
|
||||
}, 1000)
|
||||
})
|
||||
|
||||
onUnmounted(() => {
|
||||
if (ttlTimer) clearInterval(ttlTimer)
|
||||
if (ttlTimer) {
|
||||
clearInterval(ttlTimer)
|
||||
ttlTimer = null
|
||||
}
|
||||
})
|
||||
|
||||
const ttlDisplay = computed(() => {
|
||||
|
||||
@@ -377,9 +377,12 @@ function onClearCli() {
|
||||
history.value = []
|
||||
}
|
||||
|
||||
let unsubscribeMessageCleanup: (() => void) | null = null
|
||||
let monitorMessageCleanup: (() => void) | null = null
|
||||
|
||||
onMounted(() => {
|
||||
window.electronAPI.redis.onSubscribeMessage(onSubscribeMessage)
|
||||
window.electronAPI.redis.onMonitorMessage(onMonitorMessage)
|
||||
unsubscribeMessageCleanup = window.electronAPI.redis.onSubscribeMessage(onSubscribeMessage)
|
||||
monitorMessageCleanup = window.electronAPI.redis.onMonitorMessage(onMonitorMessage)
|
||||
window.addEventListener('clear-cli', onClearCli)
|
||||
})
|
||||
|
||||
@@ -387,6 +390,8 @@ onUnmounted(() => {
|
||||
if (connStore.activeId && streamingMode.value !== 'none') {
|
||||
stopStreaming()
|
||||
}
|
||||
unsubscribeMessageCleanup?.()
|
||||
monitorMessageCleanup?.()
|
||||
window.removeEventListener('clear-cli', onClearCli)
|
||||
})
|
||||
</script>
|
||||
|
||||
@@ -132,10 +132,14 @@ export const useKeyStore = defineStore('key', () => {
|
||||
selectedTTL.value = ttl
|
||||
}
|
||||
|
||||
let loadKeyVersion = 0
|
||||
|
||||
async function loadKeyData(connId: string, key: string) {
|
||||
const currentVersion = ++loadKeyVersion
|
||||
loading.value = true
|
||||
try {
|
||||
await loadKeyTypeAndTTL(connId, key)
|
||||
if (currentVersion !== loadKeyVersion) return
|
||||
selectedKey.value = key
|
||||
|
||||
// Key no longer exists — show expired state and refresh list
|
||||
@@ -160,29 +164,35 @@ export const useKeyStore = defineStore('key', () => {
|
||||
break
|
||||
case 'hash': {
|
||||
const result = await window.electronAPI.redis.hashScan(connId, key, '0', 100)
|
||||
if (currentVersion !== loadKeyVersion) return
|
||||
keyData.value = result.fields
|
||||
break
|
||||
}
|
||||
case 'list': {
|
||||
const len = await window.electronAPI.redis.listLength(connId, key)
|
||||
if (currentVersion !== loadKeyVersion) return
|
||||
keyData.value = { length: len }
|
||||
break
|
||||
}
|
||||
case 'set': {
|
||||
const size = await window.electronAPI.redis.setSize(connId, key)
|
||||
if (currentVersion !== loadKeyVersion) return
|
||||
keyData.value = { size }
|
||||
break
|
||||
}
|
||||
case 'zset': {
|
||||
const size = await window.electronAPI.redis.zsetSize(connId, key)
|
||||
if (currentVersion !== loadKeyVersion) return
|
||||
keyData.value = { size }
|
||||
break
|
||||
}
|
||||
case 'stream': {
|
||||
try {
|
||||
const info = await window.electronAPI.redis.streamInfo(connId, key)
|
||||
if (currentVersion !== loadKeyVersion) return
|
||||
keyData.value = { length: info.length }
|
||||
} catch {
|
||||
if (currentVersion !== loadKeyVersion) return
|
||||
keyData.value = { length: 0 }
|
||||
}
|
||||
break
|
||||
@@ -195,6 +205,7 @@ export const useKeyStore = defineStore('key', () => {
|
||||
// command (e.g. key was overwritten). Re-fetch the type so the
|
||||
// correct editor is shown.
|
||||
await loadKeyTypeAndTTL(connId, key)
|
||||
if (currentVersion !== loadKeyVersion) return
|
||||
keyData.value = null
|
||||
}
|
||||
} finally {
|
||||
|
||||
Reference in New Issue
Block a user