René's URL Explorer Experiment

Title: fix: Resolve timer bugs causing lost timers and resource leaks by adrian-niculescu · Pull Request #1894 · NativeScript/android · GitHub

Open Graph Title: fix: Resolve timer bugs causing lost timers and resource leaks by adrian-niculescu · Pull Request #1894 · NativeScript/android

X Title: fix: Resolve timer bugs causing lost timers and resource leaks by adrian-niculescu · Pull Request #1894 · NativeScript/android

Description: This PR fixes five bugs in Timers.cpp that could cause timers to be lost, file descriptors to leak, and memory to accumulate. 1. Inverted EAGAIN retry condition (line 162) When the pipe buffer fills up and write() returns EAGAIN, the retry loop had the wrong condition: // Before - wrong: continues if timer IS deleted while (!stopped && deletedTimers_.find(timer->id) != deletedTimers_.end() && ...) // After - correct: continues if timer is NOT deleted while (!stopped && deletedTimers_.find(timer->id) == deletedTimers_.end() && ...) This caused live timers to be silently dropped when the pipe was full, because the loop never executed for timers that weren't deleted. Additionally, if a timer is cancelled while waiting for buffer space, the fix now cleans up deletedTimers_ to prevent leaking the timer ID: // After the retry loop if (deletedTimers_.find(timer->id) != deletedTimers_.end()) { deletedTimers_.erase(timer->id); } 2. File descriptor leak (line 192) Destroy() was closing only the read end of the pipe: close(fd_[0]); // fd_[1] never closed - leaked one FD per isolate lifetime Now both ends are properly closed: close(fd_[0]); close(fd_[1]); 3. deletedTimers_ accumulation (line 323) Immediate timers (delay ≤ 0) skip the sortedTimers_ queue and write directly to the pipe. If cleared before the callback runs, their IDs were added to deletedTimers_ but never removed. Added cleanup in PumpTimerLoopCallback: auto it = thiz->timerMap_.find(timerId); if (it != thiz->timerMap_.end()) { // ... execute callback ... } else { // Timer was cleared before callback ran - clean up std::lock_guard lock(thiz->mutex); thiz->deletedTimers_.erase(timerId); } 4. Data race on stopped flag (Timers.h line 131) The stopped flag was written under mutex in Destroy() but read without synchronization in PumpTimerLoopCallback(), causing undefined behavior per C++ standard. // Before bool stopped = false; // After std::atomic stopped = false; 5. Non-EAGAIN write errors drop timers (line 92) The condition if (result != -1 || errno != EAGAIN) treated all non-EAGAIN errors (EPIPE, EBADF, EINTR) as success, disabling fallback scheduling: // Before - wrong: treats EPIPE/EBADF as success if (result != -1 || errno != EAGAIN) { needsScheduling = false; // timer lost! } // After - correct: only success disables scheduling if (result != -1) { needsScheduling = false; } else if (errno == EAGAIN) { isBufferFull = true; } // Other errors: needsScheduling stays true, timer goes to sortedTimers_ Additional fix: isBufferFull reset guard (line 171) The isBufferFull optimization flag was being reset even when write() failed with non-EAGAIN errors: // Before - resets on any non-EAGAIN result } else if (isBufferFull.load() && ...) { isBufferFull = false; } // After - only resets on actual success } else if (result != -1 && isBufferFull.load() && ...) { isBufferFull = false; } This prevents incorrectly re-enabling the immediate timer optimization when the pipe is broken.

Open Graph Description: This PR fixes five bugs in Timers.cpp that could cause timers to be lost, file descriptors to leak, and memory to accumulate. 1. Inverted EAGAIN retry condition (line 162) When the pipe buffer fill...

X Description: This PR fixes five bugs in Timers.cpp that could cause timers to be lost, file descriptors to leak, and memory to accumulate. 1. Inverted EAGAIN retry condition (line 162) When the pipe buffer fill...

Opengraph URL: https://github.com/NativeScript/android/pull/1894

X: @github

direct link

Domain: patch-diff.githubusercontent.com

Links:

Viewport: width=device-width