[ruby] refactor flaky test and expose cancel_with_status (grpc#37410)

Fixes grpc#37234

Following up on the problem described in grpc#36903, there are a number of paths in `client_server_spec.rb` and a few other tests where client call objects can leak due to RPC lifecycles not being properly completed, leading to a thread not terminating.

Some of the tests, which don't use the surface-level APIs, are changed to manually close calls (and not rely on GC which might not happen before shutdown of ruby threads). `client_server_spec.rb` is updated to use surface level APIs, which manages call lifecycles correctly (this also improves the test's fidelity).

While we're here: expose `cancel_with_status` on call operations. This was only accidentally private so far. The test refactoring caught it.

Closes grpc#37410

COPYBARA_INTEGRATE_REVIEW=grpc#37410 from apolcyn:fix_call_leak b230472
PiperOrigin-RevId: 660430463

src/ruby/lib/grpc/generic/active_call.rb

@@ -44,8 +44,8 @@ class ActiveCall # rubocop:disable Metrics/ClassLength
     include Core::CallOps
     extend Forwardable
     attr_reader :deadline, :metadata_sent, :metadata_to_send, :peer, :peer_cert
-    def_delegators :@call, :cancel, :metadata, :write_flag, :write_flag=,
-                   :trailing_metadata, :status
+    def_delegators :@call, :cancel, :cancel_with_status, :metadata,
+                   :write_flag, :write_flag=, :trailing_metadata, :status
 
     # client_invoke begins a client invocation.
     #
@@ -620,6 +620,8 @@ def maybe_finish_and_close_call_locked
     # @param metadata [Hash] metadata to be sent to the server. If a value is
     # a list, multiple metadata for its key are sent
     def start_call(metadata = {})
+      # TODO(apolcyn): we should cancel and clean up the call in case this
+      # send initial MD op fails.
       merge_metadata_to_send(metadata) && send_initial_metadata
     end
 
@@ -665,9 +667,10 @@ def initialize(wrapped)
 
     # Operation limits access to an ActiveCall's methods for use as
     # a Operation on the client.
-    Operation = view_class(:cancel, :cancelled?, :deadline, :execute,
-                           :metadata, :status, :start_call, :wait, :write_flag,
-                           :write_flag=, :trailing_metadata)
+    # TODO(apolcyn): expose peer getter
+    Operation = view_class(:cancel, :cancel_with_status, :cancelled?, :deadline,
+                           :execute, :metadata, :status, :start_call, :wait,
+                           :write_flag, :write_flag=, :trailing_metadata)
 
     # InterceptableView further limits access to an ActiveCall's methods
     # for use in interceptors on the client, exposing only the deadline

src/ruby/spec/call_spec.rb

@@ -90,88 +90,101 @@
 
   describe '#status' do
     it 'can save the status and read it back' do
-      call = make_test_call
-      sts = Struct::Status.new(OK, 'OK')
-      expect { call.status = sts }.not_to raise_error
-      expect(call.status).to eq(sts)
+      make_test_call do |call|
+        sts = Struct::Status.new(OK, 'OK')
+        expect { call.status = sts }.not_to raise_error
+        expect(call.status).to eq(sts)
+      end
     end
 
     it 'must be set to a status' do
-      call = make_test_call
-      bad_sts = Object.new
-      expect { call.status = bad_sts }.to raise_error(TypeError)
+      make_test_call do |call|
+        bad_sts = Object.new
+        expect { call.status = bad_sts }.to raise_error(TypeError)
+      end
     end
 
     it 'can be set to nil' do
-      call = make_test_call
-      expect { call.status = nil }.not_to raise_error
+      make_test_call do |call|
+        expect { call.status = nil }.not_to raise_error
+      end
     end
   end
 
   describe '#metadata' do
     it 'can save the metadata hash and read it back' do
-      call = make_test_call
-      md = { 'k1' => 'v1',  'k2' => 'v2' }
-      expect { call.metadata = md }.not_to raise_error
-      expect(call.metadata).to be(md)
+      make_test_call do |call|
+        md = { 'k1' => 'v1',  'k2' => 'v2' }
+        expect { call.metadata = md }.not_to raise_error
+        expect(call.metadata).to be(md)
+      end
     end
 
     it 'must be set with a hash' do
-      call = make_test_call
-      bad_md = Object.new
-      expect { call.metadata = bad_md }.to raise_error(TypeError)
+      make_test_call do |call|
+        bad_md = Object.new
+        expect { call.metadata = bad_md }.to raise_error(TypeError)
+      end
     end
 
     it 'can be set to nil' do
-      call = make_test_call
-      expect { call.metadata = nil }.not_to raise_error
+      make_test_call do |call|
+        expect { call.metadata = nil }.not_to raise_error
+      end
     end
   end
 
   describe '#set_credentials!' do
     it 'can set a valid CallCredentials object' do
-      call = make_test_call
-      auth_proc = proc { { 'plugin_key' => 'plugin_value' } }
-      creds = GRPC::Core::CallCredentials.new auth_proc
-      expect { call.set_credentials! creds }.not_to raise_error
+      make_test_call do |call|
+        auth_proc = proc { { 'plugin_key' => 'plugin_value' } }
+        creds = GRPC::Core::CallCredentials.new auth_proc
+        expect { call.set_credentials! creds }.not_to raise_error
+      end
     end
   end
 
   describe '#cancel' do
     it 'completes ok' do
-      call = make_test_call
-      expect { call.cancel }.not_to raise_error
+      make_test_call do |call|
+        expect { call.cancel }.not_to raise_error
+      end
     end
 
     it 'completes ok when the call is closed' do
-      call = make_test_call
-      call.close
-      expect { call.cancel }.not_to raise_error
+      make_test_call do |call|
+        call.close
+        expect { call.cancel }.not_to raise_error
+      end
     end
   end
 
   describe '#cancel_with_status' do
     it 'completes ok' do
-      call = make_test_call
-      expect do
-        call.cancel_with_status(0, 'test status')
-      end.not_to raise_error
-      expect do
-        call.cancel_with_status(0, nil)
-      end.to raise_error(TypeError)
+      make_test_call do |call|
+        expect do
+          call.cancel_with_status(0, 'test status')
+        end.not_to raise_error
+        expect do
+          call.cancel_with_status(0, nil)
+        end.to raise_error(TypeError)
+      end
     end
 
     it 'completes ok when the call is closed' do
-      call = make_test_call
-      call.close
-      expect do
-        call.cancel_with_status(0, 'test status')
-      end.not_to raise_error
+      make_test_call do |call|
+        call.close
+        expect do
+          call.cancel_with_status(0, 'test status')
+        end.not_to raise_error
+      end
     end
   end
 
   def make_test_call
-    @ch.create_call(nil, nil, 'phony_method', nil, deadline)
+    call = @ch.create_call(nil, nil, 'phony_method', nil, deadline)
+    yield call
+    call.close
   end
 
   def deadline

src/ruby/spec/channel_spec.rb

@@ -118,7 +118,8 @@ def construct_with_args(a)
       deadline = Time.now + 5
 
       blk = proc do
-        ch.create_call(nil, nil, 'phony_method', nil, deadline)
+        call = ch.create_call(nil, nil, 'phony_method', nil, deadline)
+        call.close
       end
       expect(&blk).to_not raise_error
     end
@@ -132,8 +133,9 @@ def construct_with_args(a)
 
       deadline = Time.now + 5
       blk = proc do
-        ch.create_call(nil, nil, 'phony_method', nil, deadline)
+        call = ch.create_call(nil, nil, 'phony_method', nil, deadline)
         STDERR.puts "#{Time.now}: created call"
+        call.close
       end
       expect(&blk).to raise_error(RuntimeError)
       STDERR.puts "#{Time.now}: finished: raises an error if called on a closed channel"