Merge pull request #7 from centosadmin/fix-deadlocks

Fix deadlocks

ChangeLog.md

@@ -1,3 +1,7 @@
+### 0.9.2 / 2018-05-04
+
+* Fix some potential deadlocks
+
 ### 0.9.1 / 2018-04-27
 
 * Fix deadlock in Client#on_ready

lib/tdlib-ruby.rb

@@ -1,6 +1,5 @@
 require 'tdlib/version'
 require 'dry/configurable'
-require 'celluloid'
 
 module TD
   extend Dry::Configurable

lib/tdlib/client.rb

@@ -58,15 +58,16 @@
 #
 #   p @me
 class TD::Client
-  TIMEOUT = 10
+  TIMEOUT = 20
 
   def initialize(td_client = TD::Api.client_create,
                  update_manager = TD::UpdateManager.new(td_client),
                  **extra_config)
     @td_client = td_client
     @update_manager = update_manager
     @config = TD.config.client.to_h.merge(extra_config)
-    @ready_condition = Celluloid::Condition.new
+    @ready_condition_mutex = Mutex.new
+    @ready_condition = ConditionVariable.new
     authorize
     @update_manager.run
   end
@@ -92,15 +93,25 @@ def broadcast(query)
   # @param [Hash] query
   # @return [Hash]
   def broadcast_and_receive(query, timeout: TIMEOUT)
-    condition = Celluloid::Condition.new
+    condition = ConditionVariable.new
     extra = TD::Utils.generate_extra(query)
-    handler = ->(update) { condition.signal(update) if update['@extra'] == extra }
+    result = nil
+    mutex = Mutex.new
+    handler = ->(update) do
+      next unless update['@extra'] == extra
+      mutex.synchronize do
+        result = update
+        condition.signal
+      end
+    end
     @update_manager.add_handler(handler)
     query['@extra'] = extra
-    TD::Api.client_send(@td_client, query)
-    condition.wait(timeout)
-  rescue Celluloid::ConditionError
-    raise TD::TimeoutError
+    mutex.synchronize do
+      TD::Api.client_send(@td_client, query)
+      condition.wait(mutex, timeout)
+      raise TD::TimeoutError if result.nil?
+      result
+    end
   end
 
   # Synchronously executes TDLib request
@@ -128,9 +139,10 @@ def on(update_type, &_)
   end
 
   def on_ready(timeout: TIMEOUT, &_)
-    yield self if @ready || @ready_condition.wait(timeout)
-  rescue Celluloid::ConditionError
-    raise TD::TimeoutError
+    @ready_condition_mutex.synchronize do
+      return(yield self) if @ready || (@ready_condition.wait(@ready_condition_mutex, timeout) && @ready)
+      raise TD::TimeoutError
+    end
   end
 
   # Stops update manager and destroys TDLib client
@@ -163,8 +175,10 @@ def authorize
         broadcast(encryption_key_query)
       else
         @update_manager.remove_handler(handler)
-        @ready = true
-        @ready_condition.signal(true)
+        @ready_condition_mutex.synchronize do
+          @ready = true
+          @ready_condition.broadcast
+        end
       end
     end
     @update_manager.add_handler(handler)

lib/tdlib/update_manager.rb

@@ -1,4 +1,6 @@
 class TD::UpdateManager
+  TIMEOUT = 30
+
   attr_reader :handlers
 
   def initialize(td_client)
@@ -34,7 +36,7 @@ def stopped?
   private
 
   def handle_update
-    update = TD::Api.client_receive(@td_client, 10)
+    update = TD::Api.client_receive(@td_client, TIMEOUT)
     @mutex.synchronize do
       @handlers.each { |h| h.call(update) } unless update.nil?
     end

lib/tdlib/version.rb

@@ -1,4 +1,4 @@
 module TD
   # tdlib-ruby version
-  VERSION = "0.9.1"
+  VERSION = "0.9.2"
 end

spec/integration/tdlib_spec.rb

@@ -22,6 +22,12 @@
 
     it { is_expected.to include(client) }
     it { is_expected.to include('ready') }
+
+    context 'when timeout reached' do
+      subject { client.on_ready(timeout: 0.0001) { [client, 'ready'] } }
+
+      it { expect { subject }.to raise_error(TD::TimeoutError) }
+    end
   end
 
   describe '#broadcast' do
@@ -46,6 +52,12 @@
     subject { client.on_ready { client.broadcast_and_receive(payload) } }
 
     it { is_expected.to include('@type', 'entities') }
+
+    context 'when timeout reached' do
+      subject { client.on_ready(timeout: 0.0001) { client.broadcast_and_receive(payload) } }
+
+      it { expect { subject }.to raise_error(TD::TimeoutError) }
+    end
   end
 
   describe '#on' do