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

Investigate "Unclosed client session" warning. #3216

Closed
willmcgugan opened this issue Sep 1, 2023 · 7 comments
Closed

Investigate "Unclosed client session" warning. #3216

willmcgugan opened this issue Sep 1, 2023 · 7 comments
Labels
Can not reproduce The issue couldn't be reproduced

Comments

@willmcgugan
Copy link
Collaborator

Reported on Discord https://discord.com/channels/1026214085173461072/1147078573526626395

Running with textual -dev can produce the following message.

Unclosed client session
client_session: <aiohttp.client.ClientSession object at 0x10435ec80>

Suggests that there is some cleanup on exit that isn't being done.

@holdenweb
Copy link
Contributor

holdenweb commented Sep 1, 2023

Please note, in case it's relevant, the reported error took place on the text-area branch (commit 7c5ab84). As I'm currently unable to replicate the bug using an Input rather than a TextArea, or with my current source I'll close this issue, and re-open should I discover it's real. [Ah, I won't be able to close the issue. Sorry].

@willmcgugan
Copy link
Collaborator Author

I think I might still investigate. There is only one point where we create a ClientSession in textual-dev, should be obvious if we aren't closing it gracefully.

@rodrigogiraoserrao
Copy link
Contributor

rodrigogiraoserrao commented Sep 21, 2023

The aiohttp documentation on ClientSession ends with a section on “Graceful Shutdown” which mentions a limitation of the underlying asyncio machinery.

It proposes a workaround by adding a small sleep between closing the session and closing the loop, and it further points links to an aiohttp issue aio-libs/aiohttp#1925 that supposedly tracks the resolution of the underlying problem, which supposedly will be solved in aiohttp 4.0.0.
In the meantime, the linked issue proposes a workaround.

The current code in textual-web doesn't seem to implement any of the proposed workarounds, so it might be that we didn't take this into account.

To test the workaround, I'll need to repro the issue. @holdenweb if you have the original code that was triggering the warning, that'd possibly help me.

@rodrigogiraoserrao
Copy link
Contributor

Got the source from the issue reporter, it's a test app (see below) and some utils from this repo.

Test app
from textual.app import App, ComposeResult
from textutils.key_value_edit import KeyValueEditScreen


class TestApp(App):

    def on_mount(self):
        self.push_screen(KeyValueEditScreen("KEY", "Value Value Value"),
                         self.done)
        self.screen.styles.background = "blue"

    def done(self, result):
        self.exit(message=f"Done!\nresult: {result}")

app = TestApp()
app.run()

@rodrigogiraoserrao
Copy link
Contributor

Couldn't reproduce on Windows/MacOs x Python 3.7, 3.8, 3.9, 3.10, 3.11.

@rodrigogiraoserrao rodrigogiraoserrao added the Can not reproduce The issue couldn't be reproduced label Sep 28, 2023
@rodrigogiraoserrao
Copy link
Contributor

Given all the documented issues showing this is a problem upstream and given that we can't reproduce it to test a workaround on Textual, I'll close this now.

@github-actions
Copy link

github-actions bot commented Oct 2, 2023

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Can not reproduce The issue couldn't be reproduced
Projects
None yet
Development

No branches or pull requests

3 participants