fix(pgsql): guard monitor task removal against self-move

[Snehil-Shah]

The build crashes at runtime when linked with libc++ due to a self-std::move operation, which is unspecified behavior. It happens to work in our favour with libstdc++, but crashes with libc++.

Ref: https://cplusplus.github.io/LWG/issue2839

---

Summary by cubic

Prevents a crash in libc++ builds by guarding against a self-move in PgSQL monitor task removal. Also fixes index validation and improves the error log.

• Bug Fixes
  • Skip move when removing the last task (avoid self-std::move).
  • Replace invalid bounds check with idx >= size; fix log text and use %zu.

^{Written for commit 1d70298. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes
  • Improved task removal validation to prevent invalid index access.
  • Corrected the associated error message.
  • Avoided unnecessary task movement when removing the final task.

[gitar-bot]

Code Review ✅ Approved

Guards PostgreSQL monitor task removal against self-move operations to prevent runtime crashes when compiled with libc++. No issues found.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[coderabbitai]

📝 Walkthrough

Walkthrough

rm_task_fast now rejects indices outside the active task range. It also replaces a removed task with the final task only when the removed task is not already final.

Changes

Task removal handling

Layer / File(s)	Summary
Validate and compact task removal lib/PgSQL_Monitor.cpp	rm_task_fast rejects indices at or beyond task_poll.size. It conditionally moves the final task into the removed slot. The error message is corrected.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: renecannao

Poem

A rabbit checks the task array tight,
Rejects bad bounds before the flight.
The last task hops when slots must close,
But skips a move when already home.
Clean little changes, swift and bright.

🚥 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 clearly and concisely describes the main change: preventing self-move during PostgreSQL monitor task removal.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[Snehil-Shah]

Drive-by correctness fix. idx is unsigned so the second check was a no-op, and idx == task_poll.size is also an invalid task index.

[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 `@lib/PgSQL_Monitor.cpp`:
- Around line 1273-1276: In the invalid-index guard around task_poll access,
make the function return unconditionally after proxy_error; retain assert(0)
only as diagnostic if desired, but ensure execution cannot reach writes to
task_poll.fds or task_poll.tasks or the task_poll.size decrement when idx >=
task_poll.size.

🪄 Autofix

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 Plus

Run ID: 2f0cc634-90f3-4c3e-80c2-aad604651a7b

📥 Commits

Reviewing files that changed from the base of the PR and between 7b0d92f and 1d70298.

📒 Files selected for processing (1)

• lib/PgSQL_Monitor.cpp

📜 Review details

⏰ Context from checks skipped due to timeout. (1)

• GitHub Check: cubic · AI code reviewer

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{cpp,h,hpp}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{cpp,h,hpp}: Class names must use PascalCase with protocol prefixes such as MySQL_, PgSQL_, and ProxySQL_.
Member variables must use snake_case.
Constants and macros must use UPPER_SNAKE_CASE.
Use C++17, and gate conditional code with #ifdef PROXYSQL31, #ifdef PROXYSQL40, #ifdef PROXYSQLFFTO, #ifdef PROXYSQLTSDB, and #ifdef PROXYSQLCLICKHOUSE; PROXYSQLGENAI must not guard core code outside plugins/genai/.
Consider performance implications when changing hot paths or other performance-critical code.
Use RAII for resource management and jemalloc for allocation.
Use pthread mutexes for synchronization and std::atomic<> for counters.

Files:

• lib/PgSQL_Monitor.cpp

[cubic-dev-ai]

All reported issues were addressed across 1 file

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[renecannao]

Thank you @Snehil-Shah for tracking down and fixing this libc++ crash.

I reviewed the change and found no actionable correctness issues. The corrected bounds check and the guard against moving the last task onto itself both look sound. A focused libc++ regression test covering last-slot and non-last-slot removal would be useful future protection, but I do not consider it merge-blocking.

Looks good from the code-review side. Thanks again for the contribution.