Skip to content

fix: Multiple memory leaks in ChatView component (ChatView_1113, ChatView_1236) #4247

@kiwina

Description

@kiwina

🔍 Summary of Issues in ChatView:

The ChatView component in webview-ui/src/components/chat/ChatView.tsx has multiple instances where asynchronous operations or setTimeout calls are not properly cleaned up if the component unmounts before their completion.

1. Auto-Scroll Timeout Leak (ChatView_1113):

  • File: webview-ui/src/components/chat/ChatView.tsx
  • Line: Original 1113
  • Summary: A useEffect hook schedules a setTimeout to call scrollToBottomSmooth(). This timeout is not cleared on unmount or effect re-run.
  • Leak: Potential execution of scrollToBottomSmooth() on an unmounted component.

2. Auto-Approve Async Delay Leak (ChatView_1236):

  • File: webview-ui/src/components/chat/ChatView.tsx
  • Line: Original 1236 (within autoApprove function)
  • Summary: The autoApprove function uses an await new Promise(setTimeout) for writeDelayMs. Subsequent state updates (setSendingDisabled, setClineAsk, setEnableButtons) can occur on an unmounted component if it unmounts during this delay.
  • Leak: State updates on an unmounted component.

📌 Why These Are Leaks:

Calling state setters or manipulating DOM refs on unmounted components is a common source of React warnings and indicates memory leaks. Pending asynchronous tasks and timeouts can hold references, preventing components from being fully garbage collected.

✅ Consolidated Fixes:

For ChatView_1113 (Auto-Scroll Timeout):
The useEffect hook at original line 1111 has been updated:

useEffect(() => {
	let timerId: NodeJS.Timeout | undefined
	if (!disableAutoScrollRef.current) {
		timerId = setTimeout(() => scrollToBottomSmooth(), 50)
	}
	return () => {
		if (timerId) {
			clearTimeout(timerId)
		}
	}
}, [groupedMessages.length, scrollToBottomSmooth])

This captures the timerId and clears it in the cleanup function.

For ChatView_1236 (Auto-Approve Async Delay):
A isMountedRef has been added to ChatViewComponent:

// At the top of ChatViewComponent (e.g., after line 62)
const isMountedRef = useRef(true)

// Add a useEffect to manage the mounted state
useEffect(() => {
	isMountedRef.current = true
	return () => {
		isMountedRef.current = false
	}
}, [])

The autoApprove function (around original line 1236) has been updated to check isMountedRef.current:

// Inside the autoApprove function, after the await (around line 1237):
const autoApprove = async () => {
	if (lastMessage?.ask && isAutoApproved(lastMessage)) {
		if (lastMessage.ask === "tool" && isWriteToolAction(lastMessage)) {
			await new Promise((resolve) => setTimeout(resolve, writeDelayMs))
			if (!isMountedRef.current) {
				return
			}
		}
		// vscode.postMessage is safe to call even if unmounted
		vscode.postMessage({ type: "askResponse", askResponse: "yesButtonClicked" })

		if (isMountedRef.current) {
			setSendingDisabled(true)
			setClineAsk(undefined)
			setEnableButtons(false)
		}
	}
}

The user's suggestion to combine state updates into a single check block for ChatView_1236 has been incorporated.

These changes ensure that asynchronous operations and timeouts are properly managed and cleaned up.

🧪 Combined Confidence: 90%

Metadata

Metadata

Assignees

No one assigned

    Labels

    Issue - In ProgressSomeone is actively working on this. Should link to a PR soon.bugSomething isn't working

    Type

    No type

    Projects

    Status

    Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions