Fix issue with race condition in server starts where servers may fail…

… to bind connections and never reach serving state (fixes #147)

CHANGELOG.md

@@ -2,6 +2,11 @@ Changelog for the gruf gem. This includes internal history before the gem was ma
 
 ### Pending release
 
+### 2.13.1
+
+- Fix issue with race condition in server starts where servers may fail to bind connections and never reach  
+  serving state (fixes #147)
+
 ### 2.13.0
 
 - Remove server mutex handling in deference to core grpc signal handling

lib/gruf/server.rb

@@ -90,7 +90,6 @@ def start!
         logger.info { "Starting gruf server at #{@hostname}..." }
         server.run_till_terminated_or_interrupted(KILL_SIGNALS)
       end
-      server.wait_till_running
       @started = true
       update_proc_title(:serving)
       server_thread.join

spec/gruf/client_spec.rb

@@ -145,7 +145,7 @@
       let(:method_name) { :GetThing }
 
       it 'calls the appropriate method with the right signature' do
-        expect(client).to receive(:get_thing).with(req, return_op: true, metadata: metadata).and_return(op)
+        expect(client).to receive(:get_thing).with(an_instance_of(::Rpc::GetThingRequest), return_op: true, metadata: metadata).and_return(op)
         expect(subject).to be_truthy
       end
 
@@ -155,7 +155,7 @@
 
         it 'passes the deadline into the call' do
           expect(client).to receive(:get_thing)
-            .with(req, return_op: true, metadata: metadata, deadline: deadline).and_return(op)
+            .with(an_instance_of(::Rpc::GetThingRequest), return_op: true, metadata: metadata, deadline: deadline).and_return(op)
           expect(subject).to be_truthy
         end
       end

spec/gruf/server_spec.rb

@@ -21,17 +21,12 @@
   subject { gruf_server }
 
   let(:options) { {} }
-  let(:gruf_server) { described_class.new(options) }
-
-  shared_context 'with stop thread mocked' do
-    let(:thread_mock) { double(Thread, join: nil) }
-
-    before do
-      allow(Thread).to receive(:new) { thread_mock }
-    end
-  end
+  let(:derived_options) { Gruf.rpc_server_options.merge(options) }
+  let(:gruf_server) { described_class.new(derived_options) }
 
   describe '#start!' do
+    subject { gruf_server.start! }
+
     let(:server_mock) do
       double(
         GRPC::RpcServer,
@@ -42,51 +37,40 @@
         stopped?: true
       )
     end
+    let(:options) { {} }
 
     context 'when valid options passed' do
-      include_context 'with stop thread mocked'
-
-      let(:options) { { pool_size: Random.rand(10) } }
+      let(:options) { super().merge(pool_size: Random.rand(10)) }
 
       it 'runs server with given overrides' do
-        expect(GRPC::RpcServer).to receive(:new)
-          .with(Gruf.rpc_server_options.merge(options)).and_return(server_mock)
-        gruf_server.start!
+        expect(::GRPC::RpcServer).to receive(:new).with(**derived_options).and_return(server_mock)
+        subject
       end
     end
 
     context 'when invalid options passed' do
-      include_context 'with stop thread mocked'
-
-      let(:options) { { random_option: Random.rand(10) } }
+      let(:options) { super().merge(random_option: Random.rand(10)) }
 
-      it 'runs server with default configuration' do
-        expect(GRPC::RpcServer).to receive(:new)
-          .with(Gruf.rpc_server_options).and_return(server_mock)
-        gruf_server.start!
+      it 'runs server with default configuration, ignoring invalid argument' do
+        core_options = derived_options.reject! { |k| k == :random_option }
+        expect(GRPC::RpcServer).to receive(:new).with(**core_options).and_return(server_mock)
+        subject
       end
     end
 
     context 'when some valid and some invalid options passed' do
-      include_context 'with stop thread mocked'
-
-      let(:options) { { random_option: Random.rand(10), pool_size: Random.rand(10) } }
+      let(:options) { super().merge(pool_size: Random.rand(10)) }
 
       it 'runs server with valid overrides only' do
-        valid_options = { pool_size: options[:pool_size] }
-        expect(GRPC::RpcServer).to receive(:new)
-          .with(Gruf.rpc_server_options.merge(valid_options)).and_return(server_mock)
-        gruf_server.start!
+        expect(GRPC::RpcServer).to receive(:new).with(**derived_options).and_return(server_mock)
+        subject
       end
     end
 
     context 'when no options passed' do
-      include_context 'with stop thread mocked'
-
       it 'runs server with default configuration' do
-        expect(GRPC::RpcServer).to receive(:new)
-          .with(Gruf.rpc_server_options).and_return(server_mock)
-        gruf_server.start!
+        expect(GRPC::RpcServer).to receive(:new).with(**derived_options).and_return(server_mock)
+        subject
       end
     end