Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

can't shutdown slackclient correctly when other async tasks/futures are running #522

Closed
4 of 9 tasks
lundjordan opened this issue Sep 30, 2019 · 6 comments
Closed
4 of 9 tasks
Labels
area:concurrency Issues and PRs related to concurrency bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented rtm-client Version: 2x
Milestone

Comments

@lundjordan
Copy link

lundjordan commented Sep 30, 2019

Description

I have a slack client that has both a RTMClient always running but periodically I also check state of something and will create a WebClient to send a message if a state condition is met. I do this via asyncio (passing async_run) to the client inits.

When I add other async futures/tasks, I can no longer shutdown the RTMClient cleanly with a SIGINT/ctrl-c. The debug logs claim the client is shutting down but it will just repeat that every time I hit ctrl-c and keep running.

I see in the source that the RTMClient will catch and handle terminate signals. Unless I'm missing something about how asyncio works, perhaps the client is not shutting down and closing properly?

What type of issue is this? (place an x in one of the [ ])

  • bug
  • enhancement (feature request)
  • question
  • documentation related
  • testing related
  • discussion

Requirements (place an x in each of the [ ])

  • I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've searched for any related issues and avoided creating a duplicate issue.

Bug Report

Filling out the following details about bugs will help us solve your issue sooner.

Reproducible in:

slackclient version: 2.2.0

python version: 3.7.4

OS version(s): macOS 10.14.6

Steps to reproduce:

I created a basic example script to simulate this behaviour. It creates a RTMClient and a simple sleep based tasks:

example.py

import asyncio
import logging
import os

import slack

logging.basicConfig(level=logging.DEBUG)
LOGGER = logging.getLogger(__name__)


async def sleepy_count(name, sleep_for):
    for i in range(10):
        await asyncio.sleep(sleep_for)
        LOGGER.debug(f"{name} - slept {i + 1} times.")


async def slack_client_and_sleeps():
    # real-time-messaging Slack client
    client = slack.RTMClient(token=os.environ['SLACK_API_TOKEN'], run_async=True)

    sleepy_count_task = asyncio.create_task(sleepy_count("first counter", 1))
    sleepy_count_task2 = asyncio.create_task(sleepy_count("second counter", 3))

    await asyncio.gather(client.start(), sleepy_count_task, sleepy_count_task2)


async def slack_client():
    # real-time-messaging Slack client
    client = slack.RTMClient(token=os.environ['SLACK_API_TOKEN'], run_async=True)

    await asyncio.gather(client.start())


async def sleeps():
    sleepy_count_task = asyncio.create_task(sleepy_count("first counter", 1))
    sleepy_count_task2 = asyncio.create_task(sleepy_count("second counter", 3))

    await asyncio.gather(sleepy_count_task, sleepy_count_task2)


if __name__ == "__main__":
    #  sigint closes program correctly
    #  asyncio.run(slack_client())

    # sigint closes program correctly
    #  asyncio.run(sleeps())

    # sigint doesn't actually close properly
    #  asyncio.run(slack_client_and_sleeps())

Expected result:

If I un-comment asyncio.run(slack_client_and_sleeps()) and run the script, I would have thought the program would close just like it does with asyncio.run(sleeps()) and asyncio.run(slack_client()) but it doesn't.

Actual result:

running the script with asyncio.run(slack_client_and_sleeps()) and sending SIGINT a few times doesn't actually stop the program:

shell

DEBUG:asyncio:Using selector: KqueueSelector
DEBUG:slack.rtm.client:Retrieving websocket info.
DEBUG:slack.web.slack_response:Received the following response: {'ok': True, # SCRUBBING CHANNEL AND WORKSPACE DETAILS ...}
DEBUG:__main__:first counter - slept 1 times.
DEBUG:slack.rtm.client:The Websocket connection has been opened.
DEBUG:__main__:first counter - slept 2 times.
^CDEBUG:slack.rtm.client:The Slack RTMClient is shutting down.
WARNING:slack.rtm.client:Websocket was closed.
DEBUG:__main__:second counter - slept 1 times.
DEBUG:__main__:first counter - slept 3 times.
^CDEBUG:slack.rtm.client:The Slack RTMClient is shutting down.

I'm then forced to kill -9ing it.

@mozartilize
Copy link

my current solution:

async def inf_loop():
    logger = logging.getLogger()
    while 1:
        try:
            logger.info("Ping Pong! I'm alive")
            await asyncio.sleep(900)
        except asyncio.CancelledError:
            break

tasks = asyncio.gather(rtm_client.start(), inf_loop())

def callback(signum, frame):
    tasks.cancel()
    logger.warning("Cancelling tasks...")

# loop.add_signal_handler(signal.SIGINT, callback)
signal.signal(signal.SIGINT, callback)
signal.signal(signal.SIGTERM, callback)
try:
    loop.run_until_complete(tasks)
except asyncio.CancelledError as e:
    logger.error(e)
finally:
    logger.info("Quitting... Bye!")
    loop.close()

it closes rtm_client immediately even though there's a ws closed message should be received from the server hence the log Websocket was closed. does not show

@stevengill
Copy link
Member

Thanks for all of the detail @mozartilize! I'm able to reproduce the bug with the code you provided.

@stevengill stevengill added the bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented label Dec 12, 2019
@RodneyU215 RodneyU215 added the area:concurrency Issues and PRs related to concurrency label Feb 8, 2020
@seratch seratch added this to the 2.6.0 milestone Apr 20, 2020
seratch added a commit to seratch/python-slack-sdk that referenced this issue Apr 21, 2020
seratch added a commit to seratch/python-slack-sdk that referenced this issue Apr 21, 2020
@seratch
Copy link
Member

seratch commented May 14, 2020

Resolving this will be out of scope in v2.6.

@seratch seratch modified the milestones: 2.6.0, 2.7.0 May 14, 2020
@seratch seratch modified the milestones: 2.7.0, 2.8.0 Jun 1, 2020
@seratch seratch modified the milestones: 3.0.0, 3.1.0 Jul 29, 2020
@seratch
Copy link
Member

seratch commented Oct 27, 2020

As of v2.9.3 and v3.0.0rc1, this issue is not yet fixed.

@seratch seratch modified the milestones: 3.1.0, 3.3.0 Nov 25, 2020
@maresb
Copy link

maresb commented Dec 14, 2020

After many hours of struggling to add the RTMClient to my existing event loop, I discovered an alternative approach of giving the client its own thread, which seems much simpler and more reliable:

import asyncio
import os
from threading import Thread
from slack import RTMClient

def start_client():

    token = os.environ['SLACK_API_TOKEN']

    # The thread will need its own event loop.
    loop = asyncio.new_event_loop()

    # Instantiate the client.
    rtm_client = RTMClient(token=token, loop=loop)

    # Prepare a new thread to run the client's start() method.
    slack_thread = Thread(target=rtm_client.start)

    # When exiting, the program should not wait for the Slack thread to finish.
    slack_thread.setDaemon(True)

    # Let's go!
    slack_thread.start()

@seratch
Copy link
Member

seratch commented Feb 3, 2021

Hi, slack-sdk v.3.3 is introducing a new RTM implementation slack_sdk.rtm.v2.RTMClient, which resolves this issue. See #932 for details.

We won't fix this issue for slack_sdk.rtm.RTMClient. We'd appreciate it if I could get your understanding about that.

@seratch seratch closed this as completed Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:concurrency Issues and PRs related to concurrency bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented rtm-client Version: 2x
Projects
None yet
Development

No branches or pull requests

6 participants