[PR #2] [MERGED] fix: prevent duplicate instance creation in gracefulReload #1

New issue

Closed

opened 2026-02-27 20:18:22 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 20:18:22 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/NodeDaemon/NodeDaemon/pull/2
Author: @ersinkoc
Created: 11/5/2025
Status: ✅ Merged
Merged: 11/5/2025
Merged by: @ersinkoc

Base: main ← Head: claude/identify-and-fix-bug-011CUoybhg5RCPhRPNAPvgXy

📝 Commits (1)

f3a7021 fix: prevent duplicate instance creation in gracefulReload

📊 Changes

2 files changed (+294 additions, -8 deletions)

View changed files

📝 src/core/ProcessOrchestrator.ts (+9 -8)
➕ tests/unit/graceful-reload-bug.test.js (+285 -0)

📄 Description

Fixes a critical bug in ProcessOrchestrator.gracefulReload where each worker reload would create duplicate instance objects.

Bug Description:
The gracefulReload method was creating instance objects and pushing them to the instances array (line 606), then calling startClusterInstance (line 609) which creates ANOTHER instance object with a different ID and pushes that too. This resulted in 2 instance objects per worker.

Root Cause:

Lines 597-606: Creates instance with generateId() and pushes to array
Line 609: Calls startClusterInstance(processInfo, i)
startClusterInstance lines 136-143: Creates NEW instance with different generateId() and pushes to array again
Result: Double the expected number of instances in the array

Impact:

2× memory usage (duplicate instance objects for every worker)
Mismatched instance IDs (one ID created, different ID tracked)
Orphaned instance objects (one never updated or monitored)
Incorrect instance count reporting
Confusion in process state management

Fix:
Modified gracefulReload to use spawnClusterWorkerForInstance() instead of startClusterInstance(). This reuses the instance object created in gracefulReload rather than creating a new one.

Before Fix:

gracefulReload creates instance → startClusterInstance creates another
4 workers = 8 instance objects (4 orphaned)

After Fix:

gracefulReload creates instance → spawnClusterWorkerForInstance updates it
4 workers = 4 instance objects (correct)

Testing:
Added comprehensive test suite in graceful-reload-bug.test.js that verifies:

No duplicate instances created during graceful reload
Correct instance count throughout reload lifecycle
Instance IDs preserved during spawn
No orphaned instances after multiple reloads
Correct instance count for all cluster sizes

All 5 new tests pass. Existing test suite shows no regressions (55/58 passing).

Location: src/core/ProcessOrchestrator.ts:586-622

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/NodeDaemon/NodeDaemon/pull/2 **Author:** [@ersinkoc](https://github.com/ersinkoc) **Created:** 11/5/2025 **Status:** ✅ Merged **Merged:** 11/5/2025 **Merged by:** [@ersinkoc](https://github.com/ersinkoc) **Base:** `main` ← **Head:** `claude/identify-and-fix-bug-011CUoybhg5RCPhRPNAPvgXy` --- ### 📝 Commits (1) - [`f3a7021`](https://github.com/NodeDaemon/NodeDaemon/commit/f3a70212c015f91cb04241c966b2c27f85f4d978) fix: prevent duplicate instance creation in gracefulReload ### 📊 Changes **2 files changed** (+294 additions, -8 deletions) <details> <summary>View changed files</summary> 📝 `src/core/ProcessOrchestrator.ts` (+9 -8) ➕ `tests/unit/graceful-reload-bug.test.js` (+285 -0) </details> ### 📄 Description Fixes a critical bug in ProcessOrchestrator.gracefulReload where each worker reload would create duplicate instance objects. **Bug Description:** The `gracefulReload` method was creating instance objects and pushing them to the instances array (line 606), then calling `startClusterInstance` (line 609) which creates ANOTHER instance object with a different ID and pushes that too. This resulted in 2 instance objects per worker. **Root Cause:** - Lines 597-606: Creates instance with generateId() and pushes to array - Line 609: Calls startClusterInstance(processInfo, i) - startClusterInstance lines 136-143: Creates NEW instance with different generateId() and pushes to array again - Result: Double the expected number of instances in the array **Impact:** - 2× memory usage (duplicate instance objects for every worker) - Mismatched instance IDs (one ID created, different ID tracked) - Orphaned instance objects (one never updated or monitored) - Incorrect instance count reporting - Confusion in process state management **Fix:** Modified gracefulReload to use `spawnClusterWorkerForInstance()` instead of `startClusterInstance()`. This reuses the instance object created in gracefulReload rather than creating a new one. **Before Fix:** - gracefulReload creates instance → startClusterInstance creates another - 4 workers = 8 instance objects (4 orphaned) **After Fix:** - gracefulReload creates instance → spawnClusterWorkerForInstance updates it - 4 workers = 4 instance objects (correct) **Testing:** Added comprehensive test suite in graceful-reload-bug.test.js that verifies: - No duplicate instances created during graceful reload - Correct instance count throughout reload lifecycle - Instance IDs preserved during spawn - No orphaned instances after multiple reloads - Correct instance count for all cluster sizes All 5 new tests pass. Existing test suite shows no regressions (55/58 passing). Location: src/core/ProcessOrchestrator.ts:586-622 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>