sync: guard semaphore grant self-move #35249

Problem

CountingSemaphoreGrant::operator=(CountingSemaphoreGrant&&) releases the currently held semaphore grant before taking ownership from the source grant.

On self-move assignment, the source and destination are the same object, so this releases the held grant and leaves the object empty.

Fix

Guard move assignment against self-move, making it a no-op when source and destination alias.

Add a regression test that verifies a self-moved grant remains held.

Test

git diff --check
cmake --build build --target test_bitcoin -j 8
build/bin/test_bitcoin --run_test=sync_tests

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35249.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

ACK 0a3c13d

This looks like a safe change to me. It makes sense to avoid self-assignment. I tested the new test on master without the change to src/semaphore_grant.h and it fails as expected.

Just a note I only see the = operator used twice for CountingSemaphoreGrant

src/net.cpp|3022 col 19| grant = CountingSemaphoreGrant<>(*semAddnode, /*fTry=*/true);
src/net.cpp|3068 col 26| pnode->grantOutbound = std::move(grant_outbound);

Tested ACK 0a3c13ddb7ddad0a51dd4d0722f09fbfe569047e

Reviewed the move-assignment change and regression test. The self-move guard preserves the existing path for distinct objects while avoiding the previous Release() on self-assignment.

Built locally with -DENABLE_IPC=OFF and ran:

• git diff --check origin/master...HEAD
• cmake --build build --target test_bitcoin -j 8
• build/bin/test_bitcoin --run_test=sync_tests
• build/bin/test_bitcoin --run_test=sync_tests/semaphore_grant_self_move
• ctest --test-dir build -R '^sync_tests$' --output-on-failure
• valgrind --error-exitcode=1 --quiet build/bin/test_bitcoin --run_test=sync_tests/semaphore_grant_self_move

I think the scenarios where we'd run into this are very construed. This doesn't seem worth the fix in my opinion, so closing this again.