fix(wecom): fix context leak in Start() and data race in processedMsgs
Cancel the constructor-created context before overwriting in Start() to prevent the original cancel function from becoming unreachable. Move len(processedMsgs) check inside the write lock to eliminate a data race, and re-insert the current msgID after map reset to prevent duplicate processing of the in-flight message. Applies to both WeComBotChannel and WeComAppChannel.
This commit is contained in:
parent
8e06e2adbd
commit
e9b4886573
2 changed files with 18 additions and 10 deletions
|
|
@ -148,6 +148,10 @@ func (c *WeComAppChannel) Name() string {
|
|||
func (c *WeComAppChannel) Start(ctx context.Context) error {
|
||||
logger.InfoC("wecom_app", "Starting WeCom App channel...")
|
||||
|
||||
// Cancel the context created in the constructor to avoid a resource leak.
|
||||
if c.cancel != nil {
|
||||
c.cancel()
|
||||
}
|
||||
c.ctx, c.cancel = context.WithCancel(ctx)
|
||||
|
||||
// Get initial access token
|
||||
|
|
@ -601,14 +605,14 @@ func (c *WeComAppChannel) processMessage(ctx context.Context, msg WeComXMLMessag
|
|||
return
|
||||
}
|
||||
c.processedMsgs[msgID] = true
|
||||
c.msgMu.Unlock()
|
||||
|
||||
// Clean up old messages periodically (keep last 1000)
|
||||
// Clean up old messages while still holding the lock to avoid a data race
|
||||
// on len(). Reset the map but re-insert the current msgID so it remains
|
||||
// deduplicated.
|
||||
if len(c.processedMsgs) > 1000 {
|
||||
c.msgMu.Lock()
|
||||
c.processedMsgs = make(map[string]bool)
|
||||
c.msgMu.Unlock()
|
||||
c.processedMsgs[msgID] = true
|
||||
}
|
||||
c.msgMu.Unlock()
|
||||
|
||||
senderID := msg.FromUserName
|
||||
chatID := senderID // WeCom App uses user ID as chat ID for direct messages
|
||||
|
|
|
|||
|
|
@ -112,6 +112,10 @@ func (c *WeComBotChannel) Name() string {
|
|||
func (c *WeComBotChannel) Start(ctx context.Context) error {
|
||||
logger.InfoC("wecom", "Starting WeCom Bot channel...")
|
||||
|
||||
// Cancel the context created in the constructor to avoid a resource leak.
|
||||
if c.cancel != nil {
|
||||
c.cancel()
|
||||
}
|
||||
c.ctx, c.cancel = context.WithCancel(ctx)
|
||||
|
||||
c.SetRunning(true)
|
||||
|
|
@ -326,14 +330,14 @@ func (c *WeComBotChannel) processMessage(ctx context.Context, msg WeComBotMessag
|
|||
return
|
||||
}
|
||||
c.processedMsgs[msgID] = true
|
||||
c.msgMu.Unlock()
|
||||
|
||||
// Clean up old messages periodically (keep last 1000)
|
||||
// Clean up old messages while still holding the lock to avoid a data race
|
||||
// on len(). Reset the map but re-insert the current msgID so it remains
|
||||
// deduplicated.
|
||||
if len(c.processedMsgs) > 1000 {
|
||||
c.msgMu.Lock()
|
||||
c.processedMsgs = make(map[string]bool)
|
||||
c.msgMu.Unlock()
|
||||
c.processedMsgs[msgID] = true
|
||||
}
|
||||
c.msgMu.Unlock()
|
||||
|
||||
senderID := msg.From.UserID
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue