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

client: GET request continues after cancelling the task #3426

Closed
MShekow opened this issue Dec 3, 2018 · 7 comments
Closed

client: GET request continues after cancelling the task #3426

MShekow opened this issue Dec 3, 2018 · 7 comments
Labels

Comments

@MShekow
Copy link

MShekow commented Dec 3, 2018

Long story short

When I cancel a task which actually GETs a large file, I expect the transfer to truly stop once I called task.cancel().

Expected behaviour

CancelledError is raised inside my co-routine - the ClientSession context manager and request context manager is left, so I expect the request to have truly finished.

Actual behaviour

The transfer continues in the background.

Steps to reproduce

import asyncio
import time
from contextlib import suppress
from threading import Thread

import aiohttp


async def request(url):
    async with aiohttp.ClientSession() as session:
        async with session.get(url) as resp:
            return await resp.text()


async def cancel_task():
    global task
    task.cancel()
    print("task.cancel() called")
    with suppress(asyncio.CancelledError):
        await task  # hangs forever ... it makes no difference if the last 3 lines of this method are not there
    print('done cancel_task()')


loop = None


def runner():
    global loop
    loop = asyncio.new_event_loop()
    asyncio.set_event_loop(loop)
    loop.run_forever()


t = Thread(target=runner)
t.start()

time.sleep(0.5)

url = 'https://speed.hetzner.de/10GB.bin'
task = asyncio.run_coroutine_threadsafe(request(url), loop)
time.sleep(2)
print("Starting cancellation")
asyncio.run_coroutine_threadsafe(cancel_task(), loop)
time.sleep(5000)

Output:

Starting cancellation
task.cancel() called

Your environment

Win10, Python 3.6.1, aiohttp 3.4.4

@aio-libs-bot
Copy link

GitMate.io thinks the contributor most likely able to help you is @asvetlov.

Possibly related issues are #2286 (ClientSession.request unexpectedly cancels task on timeout), #1879 (Client: _request.timeout cancels the current AND the next request), #253 (Connection not closed when request is cancelled), #1861 (Client request timeout parameters are inconsistent), and #2755 (Client request tracing. Allow to get the request body in the trace.).

@asvetlov
Copy link
Member

asvetlov commented Dec 4, 2018

Could you rework the example to use the single event loop in the main thread (with a couple of tasks maybe)?

@MShekow
Copy link
Author

MShekow commented Dec 5, 2018

Here is an example with just one thread. The same result happens. I'm not sure why "a couple of tasks" (in other words, a more complex example) would be more illustrative than a minimal example.

import asyncio
import time
from contextlib import suppress

import aiohttp


async def request(url):
    async with aiohttp.ClientSession() as session:
        async with session.get(url) as resp:
            return await resp.text()


async def cancel_task(task):
    await asyncio.sleep(5)
    task.cancel()
    print("task.cancel() called")
    with suppress(asyncio.CancelledError):
        await task  # hangs forever ... it makes no difference if the last 3 lines of this method are not there
    print('done cancel_task()')


loop = asyncio.get_event_loop()
url = 'https://speed.hetzner.de/10GB.bin'
task = asyncio.ensure_future(request(url))
asyncio.ensure_future(cancel_task(task))
loop.run_forever()

Note that if I replace loop.run_forever() with

try:
    loop.run_until_complete(task)
except asyncio.CancelledError:
    pass
print("done")
time.sleep(5)

Then the transfer truly stops (during the last 5 idle seconds of keeping the Python process alive), but this is simply a side effect of stopping the event loop. Replacing time.sleep with loop.run_forever() will cause the download to resume again in the background, without any task being active.

@khazhyk
Copy link

khazhyk commented Jan 25, 2019

I am seeing this behavior as well under similar conditions, and am finding it rather problematic.

Doing something to the order of:

     async with sess.get(url, timeout=30) as resp:
            resp.raise_for_status()
            if int(resp.headers['Content-Length']) >= 50<<20:
                raise errors.BadArgument("File too big")

The file continues to be downloaded in the background

8504117 does not seem to fix it

@khazhyk
Copy link

khazhyk commented Jan 25, 2019

Simple reproducer

On windows, with aiohttp@master, and aiohttp 3.5.4, shows task continues to download (unmistakable, multiple Mbps...) even after exception raised

import aiohttp
import asyncio

BIG_URL = "https://planet.openstreetmap.org/planet/full-history/history-latest.osm.bz2"

async def get_abort_file():
    async with aiohttp.ClientSession() as sess:
         async with sess.get(BIG_URL, timeout=30) as resp:
            raise Exception("Raising exception immediately...")


loop = asyncio.get_event_loop()


async def amain():
    try:
        await get_abort_file()
    except:
        pass

    await asyncio.sleep(999999)
loop.run_until_complete(amain())

Workaround I guess, if you have a resp object, resp.connection.transport.abort()

This also seems to work, albeit a bit clunky. I don't see anywhere else this is exposed in 3.5.4

aiohttp.ClientSession(
            connector=aiohttp.TCPConnector(enable_cleanup_closed=True))

@jacobsvante
Copy link

Perhaps this deserves a separate issue, but I'll start here since it's the closest thing I could find in the issues page.

I'm experiencing this while await resp.text() is executing inside a task and I actively cancel via Cmd-Z. The stack trace I get is:

Traceback (most recent call last):
  File "./app/requests.py", line 56, in _make_request
    body = await response.text()
  File "/usr/local/lib/python3.7/site-packages/aiohttp/client_reqrep.py", line 1009, in text
    await self.read()
  File "/usr/local/lib/python3.7/site-packages/aiohttp/client_reqrep.py", line 973, in read
    self._body = await self.content.read()
  File "/usr/local/lib/python3.7/site-packages/aiohttp/streams.py", line 358, in read
    block = await self.readany()
  File "/usr/local/lib/python3.7/site-packages/aiohttp/streams.py", line 380, in readany
    await self._wait('readany')
  File "/usr/local/lib/python3.7/site-packages/aiohttp/streams.py", line 296, in _wait
    await waiter
concurrent.futures._base.CancelledError

@Dreamsorcerer
Copy link
Member

#3426 (comment)
Doesn't seem to reproduce for me on current versions. I'm assuming this is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants