Fix block_for_lock.

On two fronts: First, it should follow standard command line semantics. Second, it should work as intended: --noblock_for_lock means the client will not wait for another command to finish, but will exit eagerly. It can be useful for preventing hanging in applications that are non-interactively calling bazel.

It should have standard startup-option semantics: the default value is accepted as a no-op or can be provided to override a previous value.

The next issue involves 2 different locks - the client lock, and the server-side command lock. This duality exists because we would like, one day, to be able to run certain commands, like info or help, at the same time, so multiple commands would need specialized locks that allow some duality but blocks others. This can only be done at the server level, so as soon as the client gets the "we're connected" grpc message from the server, it releases the client lock and lets the server manage  multiple requests.

There are basically 3 possible states that are relevant to this option:

1) no other client is active, so no one holds the client lock or the command lock - the server can be used, shutdown or started as needed. - no blocking, but no need to block, either, so we're safe
2) another client (client1) holds the lock, but it is currently using a server that we want to reuse. If client1 still holds the client lock, we fail fast. Same thing if client1 is holding the server-side lock: we will exit gracefully when the BlazeCommandDispatcher responds with a failure.
3) client1 holds the lock but its server cannot be reused. (batch clients also fall into this category, as there is no server to reuse - but in this case, the client lock is still in play). However, for server mode, this is broken - the following happens:
   - Server is occupied with client1's request, holds the command lock
   - client2 wants to restart the server, so sends the old server a "shutdown" command
   - the BlazeCommandDispatcher says - nuh-uh, this is busy, and you said you didn't want to wait for the lock
   - client2 absorbs this response
   - waits (blocks...)
   - for a minute
   - then force shuts-down the old server.

So we had 2 problems - we block, and we shutdown a server that we truly intended to keep going. Now, if the server responds saying another action is using it, the client will exit correctly, and leave the old server to do its thing.

RELNOTES: None.
PiperOrigin-RevId: 205671817

1 files changed, 4 insertions, 0 deletions

diff --git a/src/test/cpp/test_util.cc b/src/test/cpp/test_util.cc
index c9e2698ee4..e39c96cd26 100644
--- a/src/test/cpp/test_util.cc
+++ b/src/test/cpp/test_util.cc
@@ -18,6 +18,10 @@
 
 namespace blaze {
 
+// TODO(b/31290599): We should not only check that the options are registered,
+// but also that they are parsed. StartupOptions* options would need to be
+// non-const to call ProcessArgs and test that the value is recognized by the
+// command line.
 void ExpectIsNullaryOption(const StartupOptions* options,
                            const std::string& flag_name) {
   EXPECT_TRUE(options->IsNullary("--" + flag_name));