src: avoid accessing EnvInst on Env destruction

[santigimeno]

I was seeing random crashes when running test/parallel/test-cli-node-options.js

Summary by CodeRabbit

• Chores
  • Enhanced internal locking mechanisms for environment management to improve stability and robustness of runtime operations.

[coderabbitai]

Walkthrough

Three environment iteration loops in N-Solid's API layer now acquire explicit EnvInst::Scope locks per environment, skip unsuccessful lock acquisitions, and pass SharedEnvInst directly to command invocation rather than using the prior pattern.

Changes

Environment Locking Refactor

Layer / File(s)	Summary
Synchronization & Iteration src/nsolid/nsolid_api.cc (1355–1367, 1396–1404, 2064–2071)	Three environment enumeration methods (PromiseTracking, UpdateTracingFlags, UpdateHasMetricsStreamHooks) now iterate using SharedEnvInst, explicitly acquire EnvInst::Scope locks per environment, skip failed lock acquisitions, and invoke RunCommand with the shared pointer instead of the prior pattern.

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 Three loops now lock with steady care,
Each SharedEnvInst paired with a scope aware,
Failed locks skip past, no crash in sight—
Safety threads now dance in the night! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: modifying environment iteration patterns to avoid accessing EnvInst during Env destruction, which directly addresses the race condition causing crashes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch santi/fix_race_envinst

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

_{Review rate limit: 4/5 reviews remaining, refill in 12 minutes.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This PR hardens EnvList’s cross-thread environment iteration during shutdown by taking an EnvInst::Scope before dereferencing each EnvInst, which matches the goal in the title/descripton of avoiding use-after-destroy access to Environment state during teardown.

Changes:

• In PromiseTracking(), switch from raw EnvInst* access to a scoped SharedEnvInst guard before reading env() and issuing the interrupt command.
• In UpdateTracingFlags() and UpdateHasMetricsStreamHooks(), add the same scope guard and skip environments whose Environment has already been cleared.
• Keep command dispatch using the already-held SharedEnvInst, avoiding a fresh lookup while destruction may be in progress.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/nsolid/nsolid_api.cc`:
- Line 1364: The loop currently dispatches commands using snapshot entry.second
which may be stale if RemoveEnv() has erased from env_map_ or env_ was nulled;
before calling RunCommand, re-check liveness by calling
EnvInst::GetInst(entry.first) and use the returned env pointer (or skip dispatch
if null) instead of the copied entry.second to avoid queuing commands to an env
in teardown; apply this pattern in the three loops that currently use
entry.second (the dispatch sites around RunCommand at the spots mentioned) so
you only call RunCommand on a live EnvInst.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9c07d711-c2dd-4cd7-a3aa-75066a2533e3

📥 Commits

Reviewing files that changed from the base of the PR and between 156ce89 and 56e11e9.

📒 Files selected for processing (1)

• src/nsolid/nsolid_api.cc