search

slack-ruby / slack-ruby-bot

The easiest way to write a Slack bot in Ruby.
MIT License
1.12k stars 187 forks source link

Raises ThreadError when started with Sinatra server #116

Open thesmart opened 7 years ago

thesmart commented 7 years ago

Starting the bot using celluloid-io is not compatible with signal trapping. I discovered this when using Puma to run a threaded Sinatra server. Sinatra has code that traps exit signals so that ctrl+c can shutdown the server.

After Sinatra shuts down, slack-ruby-bot gem calls stop! which results in an ThreadError deep inside celluloid:

/vendor/bundle/ruby/2.3.0/gems/celluloid-essentials-0.20.5/lib/celluloid/internals/uuid.rb:21:in `synchronize': can't be called from trap context (ThreadError)
    from ~/bot/vendor/bundle/ruby/2.3.0/gems/celluloid-essentials-0.20.5/lib/celluloid/internals/uuid.rb:21:in `generate'
    from ~/bot/vendor/bundle/ruby/2.3.0/gems/celluloid-0.17.3/lib/celluloid.rb:92:in `uuid'
    from ~/bot/vendor/bundle/ruby/2.3.0/gems/celluloid-0.17.3/lib/celluloid/mailbox.rb:17:in `initialize'
    from ~/bot/vendor/bundle/ruby/2.3.0/gems/celluloid-0.17.3/lib/celluloid.rb:87:in `new'
    from ~/bot/vendor/bundle/ruby/2.3.0/gems/celluloid-0.17.3/lib/celluloid.rb:87:in `mailbox'
    from ~/bot/vendor/bundle/ruby/2.3.0/gems/celluloid-0.17.3/lib/celluloid/proxy/sync.rb:20:in `method_missing'
    from ~/bot/vendor/bundle/ruby/2.3.0/gems/slack-ruby-client-0.7.8/lib/slack/real_time/client.rb:68:in `started?'
    from ~/bot/vendor/bundle/ruby/2.3.0/gems/slack-ruby-client-0.7.8/lib/slack/real_time/client.rb:63:in `stop!'
    from ~/bot/vendor/bundle/ruby/2.3.0/gems/slack-ruby-bot-0.9.0/lib/slack-ruby-bot/server.rb:48:in `stop!'
    from ~/bot/vendor/bundle/ruby/2.3.0/gems/slack-ruby-bot-0.9.0/lib/slack-ruby-bot/server.rb:91:in `block (2 levels) in handle_signals'
    from ~/bot/vendor/bundle/ruby/2.3.0/gems/sinatra-1.4.8/lib/sinatra/base.rb:1527:in `block (2 levels) in setup_traps'
    from ~/bot/vendor/bundle/ruby/2.3.0/gems/puma-3.7.0/lib/puma/single.rb:106:in `join'
    from ~/bot/vendor/bundle/ruby/2.3.0/gems/puma-3.7.0/lib/puma/single.rb:106:in `run'
    from ~/bot/vendor/bundle/ruby/2.3.0/gems/puma-3.7.0/lib/puma/launcher.rb:171:in `run'
    from ~/bot/vendor/bundle/ruby/2.3.0/gems/puma-3.7.0/lib/rack/handler/puma.rb:58:in `run'
    from ~/bot/vendor/bundle/ruby/2.3.0/gems/sinatra-1.4.8/lib/sinatra/base.rb:1506:in `start_server'
    from ~/bot/vendor/bundle/ruby/2.3.0/gems/sinatra-1.4.8/lib/sinatra/base.rb:1444:in `run!'
    from run.rb:43:in `<main>'

I believe that slack-ruby-bot is using Signal.trap improperly, and maybe needs to use Celluloid's special trap_exit method to be notified when to stop! There may be a similar handler via celluloid-io such that Signal.trap does not need to be used directly.

https://github.com/celluloid/celluloid/blob/41ff4713dac7d40b24ed6cc8906e20c23c5c6de3/spec/shared/actor_examples.rb#L582

dblock commented 7 years ago

Seems like a legit issue.

thesmart commented 7 years ago

Might be an issue with Celluloid, not the bot or client lib. If celluloid switched to a trapped run loop, the mutex in their UUID class could occur outside the trap Proc and thus avoid the ThreadError.

noqcks commented 6 years ago

I'm having this issue as well.