search

aio-libs / aiohttp

Asynchronous HTTP client/server framework for asyncio and Python
https://docs.aiohttp.org
Other
15.14k stars 2.02k forks source link

ValueError: I/O operation on closed file on WSL #8397

Closed ajkessel closed 2 months ago

ajkessel commented 6 months ago

Describe the bug

Using aiohttp under WSL as a dependency of maubot. I always get an error "ValueError: I/O operation on closed file" when attempting to upload a file.

To Reproduce

When running Maubot's mbc upload function from a WSL host box, the following error appears:

Fatal error running command
Traceback (most recent call last):
  File "/home/user/src/matrix/maubot/lib/python3.11/site-packages/maubot/cli/cliq/cliq.py", line 99, in wrapper
    asyncio.run(res)
  File "/usr/lib/python3.11/asyncio/runners.py", line 190, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/asyncio/base_events.py", line 653, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/home/user/src/matrix/maubot/lib/python3.11/site-packages/maubot/cli/cliq/cliq.py", line 55, in wrapper
    return await func(*args, sess=sess, server=server, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/user/src/matrix/maubot/lib/python3.11/site-packages/maubot/cli/commands/upload.py", line 37, in upload
    await upload_file(sess, file, server)
  File "/home/user/src/matrix/maubot/lib/python3.11/site-packages/maubot/cli/commands/upload.py", line 43, in upload_file
    async with sess.post(url, data=file, headers=headers) as resp:
  File "/home/user/src/matrix/maubot/lib/python3.11/site-packages/aiohttp/client.py", line 1194, in __aenter__
    self._resp = await self._coro
                 ^^^^^^^^^^^^^^^^
  File "/home/user/src/matrix/maubot/lib/python3.11/site-packages/aiohttp/client.py", line 545, in _request
    req = self._request_class(
          ^^^^^^^^^^^^^^^^^^^^
  File "/home/user/src/matrix/maubot/lib/python3.11/site-packages/aiohttp/client_reqrep.py", line 334, in __init__
    self.update_body_from_data(data)
  File "/home/user/src/matrix/maubot/lib/python3.11/site-packages/aiohttp/client_reqrep.py", line 559, in update_body_from_data
    size = body.size
           ^^^^^^^^^
  File "/home/user/src/matrix/maubot/lib/python3.11/site-packages/aiohttp/payload.py", line 377, in size
    return os.fstat(self._value.fileno()).st_size - self._value.tell()
                    ^^^^^^^^^^^^^^^^^^^^
ValueError: I/O operation on closed file

The following patch eliminates the error and everything then works fine:

--- src/matrix/maubot/lib/python3.10/site-packages/aiohttp/payload.py   2024-03-14 07:50:46.523398427 -0400
+++ payload.py  2024-05-02 12:08:57.973322325 -0400
@@ -375,7 +375,7 @@
     def size(self) -> Optional[int]:
         try:
             return os.fstat(self._value.fileno()).st_size - self._value.tell()
-        except OSError:
+        except:
             # data.fileno() is not supported, e.g.
             # io.BufferedReader(io.BytesIO(b'data'))
             return None

I haven't dug into this much but it seems like os.fstat is returning an error that aiohttp doesn't expect when run from a WSL device. Could you broaden the except clause to at least include ValueError? Or is there some other more fundamental fix needed in Python's os library?

Expected behavior

aiohttp should fail gracefully

Logs/tracebacks

See error message in problem description.

Python Version

Python 3.11.4

aiohttp Version

Version: 3.9.5

multidict Version

Version: 6.0.5

yarl Version

Version: 1.9.4

OS

Windows Subsystem for Linux (WSL)

Related component

Client

Additional context

N/A

Code of Conduct

Dreamsorcerer commented 6 months ago

@webknjaz Looks like the same error in your PR. https://github.com/aio-libs/aiohttp/actions/runs/8932413272/job/24536311770#step:11:6511

webknjaz commented 6 months ago

@Dreamsorcerer that seems to be happening in a different place, though..

Dreamsorcerer commented 2 months ago

@ajkessel What is the file variable in your traceback? It appears the error actually comes from file.fileno(), so I'll need to know what that object is.

Dreamsorcerer commented 2 months ago

Actually, looks like it may just be a regular file object:

>>> f = open("README.rst")
>>> f.fileno()
3
>>> f.close()
>>> f.fileno()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: I/O operation on closed file

But, I guess the question is why is it closed when in your code...

Dreamsorcerer commented 2 months ago

And how can catching that exception fix your application? If the file is closed, then the .read() call when you try to write the payload will also fail..

Dreamsorcerer commented 2 months ago

I'll need a some more info that helps answer some of that before deciding what should be changed here. Currently, I can't see how catching that exception would help.

ajkessel commented 2 months ago

This is several months old and I'm not able to replicate this currently. I'm not sure what's changed but I've been through multiple WSL and Ubuntu upgrades since I reported this bug. I suppose if no one else is seeing it, we could just close it, unless you want to be more forgiving about the types of exceptions that can be caught (or give the user more info when a filesystem exception is raised).

Dreamsorcerer commented 2 months ago

unless you want to be more forgiving about the types of exceptions that can be caught (or give the user more info when a filesystem exception is raised).

I'm just not clear what we could realistically change. Catching the error when the file is closed surely makes no difference as it will just fail again at the next step with the exact same error. We can't do anything useful with a closed file object.