[wenzeslaus]

The forkserver start method test fails intermittently in CI, including on main, so the pytest runs are flaky. The RPC servers of grass.pygrass and grass.temporal release their lock twice: when handling the stop message from the client, and in the fatal error handler of a server that the checker thread restarts at exit. This removes the extra releases, stops the servers before multiprocessing terminates them at exit, and keeps the checker thread from restarting them during shutdown.

Details: the two failures, the changes, how it was tested, and the commit message (AI generated)

Failures:

• To end a server process, the client sends it a stop message (RPCDefs.STOP). The server's handler for it, _stop, called lock.release() inside with lock:, so the lock was released twice and multiprocessing printed a ValueError traceback. The client terminated the server right after sending the message, so this showed only when the server handled the message first.
• At exit, multiprocessing terminated the daemonic servers before stop_subprocesses of grass.temporal ran. The checker thread then restarted the server without a session, it failed in G_gisinit(), and its error handler released a lock it did not hold. This is the failure in the two latest pytest runs on main (2d45381, b587a90).

Changes:

• Remove lock.release() from _stop and from the error handlers of both servers; the client never uses the lock.
• RPCServerBase.stop() waits up to 5 s for the server to exit after the stop message before terminating it, as Messenger.stop() does. This makes the test cover the shutdown path in every run.
• grass.temporal imports multiprocessing.util before registering stop_subprocesses. The import registers the multiprocessing exit handler, and exit handlers run in reverse order of registration, so stop_subprocesses now runs first.
• The checker thread does not restart a server once stop was requested. When the server was found dead and multiprocessing.util.is_exiting() is true (looked up with a fallback, as it is undocumented), _check_restart_server raises FatalError instead of restarting it, and the checker thread ends on that error. Checking the flag under the lock after is_alive() makes this race-free, because multiprocessing sets the flag before it terminates the servers. Restarting would leave a server that is never stopped, and skipping the restart without an error would make the caller wait for a reply forever. This covers servers that are never stopped explicitly and code that puts the multiprocessing exit handler first again: the first call to multiprocessing.get_logger() unregisters and re-registers it.
• A new test checks that a server terminated at exit is not restarted.

Follow-up: the per-server lock protects nothing, because the client never acquires it and each server is single-threaded. Removing it would remove this class of bugs, including the semaphore workaround in RPCServerBase.stop() and the leaked-semaphore assertion in the test.

Testing (local, Python 3.12):

• The forkserver test passed all local runs over the revisions of the change, including 50 runs under CPU load with the final one; it failed every time when the _stop release was put back.
• The new test fails with the previous code, which restarts the server.
• With the exit window forced open, no restarts or errors, also after calling multiprocessing.get_logger().
• The whole pytest suite passes except 14 tests which fail the same way without this change (local environment).
• stop() takes about 0.2 s with or without the wait (the checker thread sleep); 20 runs of t.list show no difference.

Commit message for squash-merge:

grass.pygrass: Fix double lock release when stopping RPC servers

The forkserver start method test failed intermittently in CI, including
on main, because the RPC servers of grass.pygrass and grass.temporal
released their lock twice.

The handler of the stop message, which the client sends to end the
server process, released the lock inside a with-lock block, so the lock
was released again when the block exited. The client terminated the
server right after sending the message, so the resulting traceback
appeared only when the server handled the message first.

At exit, multiprocessing terminated the servers before grass.temporal
stopped them. The checker thread then restarted a server without a
session, the server failed in G_gisinit(), and its fatal error handler
released a lock it did not hold.

Remove the lock release from the stop and fatal error handlers. Wait
for the server to exit after the stop message before terminating it.
Import multiprocessing.util in grass.temporal before registering its
exit function so that it runs before the exit handler of
multiprocessing. Do not restart a server from the checker thread once
stop was requested, and raise FatalError instead of restarting a server
that multiprocessing terminated at exit, because the restarted server
would never be stopped. Check for this only after the server was found
dead, under the lock, which is race-free because multiprocessing sets
its exiting flag before it terminates the servers.

The failures were identified and the fix written with AI assistance
(Claude Code with Opus 5.5, reviewed by Fable 5.1).

The failures were identified and the fix written with AI assistance (Claude Code with Opus 5.5, reviewed by Fable 5.1).

[petrasovaa]

I am fine with merging now, but there are 2 points you can consider:

1. The is_exiting() guard can still lose a race (python/grass/pygrass/rpc/base.py, thread_checker)

The guard is checked, the lock is released, and only then does _check_restart_server look at is_alive(). If the main thread enters multiprocessing's _exit_function in between, the server is terminated and the checker restarts it without a session. This only affects the fallback path the description mentions (servers never stopped, or get_logger() reordering the handlers), and the window is tiny, but it can be closed completely: _exit_function sets _exiting = True before it terminates any child, so moving the is_exiting() check inside _check_restart_server, under the lock and after is_alive() returned false, makes the check and the restart decision atomic. It also covers the main-thread callers of check_server(), and Messenger._check_restart_server could get the same two lines.

2. The per-server lock protects nothing (python/grass/pygrass/rpc/__init__.py)

Every bug fixed here exists only because a Lock is created per server and passed to the server function, while the client never acquires it and each server is single-threaded. Dropping the lock from the server functions and from start_server/stop would remove the whole class of bug, including the "process still starting up reopens the lock semaphore by name" workaround and the leaked-semaphore assertion in the test. Fine as a follow-up, but worth noting in the PR.

[wenzeslaus]

I did point 1, but raising FatalError instead of just not restarting. The two rejected alternatives which come from an AI analysis:

• Skip the restart: multiprocessing terminates the server at exit, then a later exit handler calls a temporal function, the request goes to the dead server, then the caller waits in recv() forever (the parent still holds the server end of the pipe).
• Restart: multiprocessing terminates the server at exit, then the checker thread or an exit handler starts a new one, then nobody stops it. Without a session, it fails in G_gisinit() (the CI failure). With a session, it keeps running after Python exits, because it holds both pipe ends and never sees EOF.

Point 2 is noted as a follow-up in the description. Hopefully, this will be fixed by some of the other efforts around this.