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

ResourceWarning: unclosed ssl.SSLSocket #454

Closed
LinusU opened this issue Jan 25, 2016 · 54 comments
Closed

ResourceWarning: unclosed ssl.SSLSocket #454

LinusU opened this issue Jan 25, 2016 · 54 comments
Assignees
Labels
feature-request This issue requests a feature. needs-review

Comments

@LinusU
Copy link

LinusU commented Jan 25, 2016

For some reason I'm getting a ResourceWarning about a unclosed socket, even when I'm specifically closing the socket myself. See testcase below:

python3 -munittest discover
import sys
import boto3
import unittest

BUCKET = ''
KEY = ''


def give_it_to_me():
    client = boto3.client('s3')
    obj = client.get_object(Bucket=BUCKET, Key=KEY)
    try:
        yield from iter(lambda: obj['Body'].read(1024), b'')
    finally:
        print('Im closing it!', file=sys.stderr, flush=True)
        obj['Body'].close()


class TestSomeShit(unittest.TestCase):
    def test_it(self):
        res = give_it_to_me()
        for chunk in res:
            pass
        print('Done', file=sys.stderr, flush=True)

Fill in any BUCKET and KEY to see the problem. Attaching my output below:

Im closing it!
test.py:22: ResourceWarning: unclosed <ssl.SSLSocket fd=7, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=6, laddr=('...', 55498), raddr=('...', 443)>
  for chunk in res:
Done
.
----------------------------------------------------------------------
Ran 1 test in 0.696s

OK
@kyleknap
Copy link
Contributor

I am able to reproduce this. Note you can only reproduce this with python3. More research needs to be done to see if it is an issue with our underlying http library or it is something with our implementation. I have found a few related python bugs, but not sure if those fit the bill especially on Python3.5 I am still seeing them:
http://bugs.python.org/issue16900
http://bugs.python.org/issue12133

@kyleknap kyleknap added the investigating This issue is being investigated and/or work is in progress to resolve the issue. label Jan 26, 2016
@kyleknap kyleknap self-assigned this Jan 26, 2016
@LinusU
Copy link
Author

LinusU commented Jan 26, 2016

Should probably mention that I'm running python 3.5 as well, 3.5.1 to be specific.

@nchammas
Copy link

I'm seeing this as well. Enabling ResourceWarning yields a lot of complaints related to unclosed sockets. Also running Python 3.5.1.

@Naddiseo
Copy link

Naddiseo commented Apr 5, 2016

I have also run into this since upgrading to python 3.5. From what I can tell it's something to with connection pooling [0]. I have found a way to "fix" the warnings, although I'm not sure it's the correct way to do so: in botocore/awsrequest.py, explicitly set the Connection: close header in the AWSRequest class:

--- awsrequest.py.old   2016-04-05 15:18:56.931892889 -0600
+++ awsrequest.py   2016-04-05 14:56:17.422863960 -0600
@@ -340,6 +340,7 @@ class AWSRequest(models.RequestEncodingM
             del kwargs['auth_path']
         models.Request.__init__(self, *args, **kwargs)
         headers = HTTPHeaders()
+        headers['Connection'] = 'close'
         if self.headers is not None:
             for key, value in self.headers.items():
                 headers[key] = value

[0] https://github.com/kennethreitz/requests/issues/2963#issuecomment-169631513

@rehmanzile
Copy link

I am also experiencing issues with python 3.5. Had to go back to 2.7.

Make sure to use the following command to install AWS cli:

pip install awscli --ignore-installed six

@jucor
Copy link

jucor commented Jan 2, 2017

Pinging this thread: is there a solution merged somewhere, please?

@robmcdan
Copy link

+1, also on python 3.5 -- any update?

@aausch
Copy link

aausch commented Apr 6, 2017

+1, seeing on python 3.6

@xiaochuan-du
Copy link

+1, seeing on python 3.6.1

@muppetjones
Copy link

+1 Still an issue. The fix given by @Naddiseo works.

@DinoBytes
Copy link

+1, seeing on python 3.6.1

@nueverest
Copy link

nueverest commented May 28, 2017

python 3.6.1 I'm testing cognito.

@diegojancic
Copy link

+1 boto3=1.4.4, python=3.6.1

@muppetjones
Copy link

+1

boto3==1.4.4
botocore==1.5.38
Python 3.5.2

@sleepdeprivation
Copy link

sleepdeprivation commented Aug 24, 2017

For future readers... I did a little research, and also clicked the link in the above discussion given by @Naddiseo and according to someone working on Requests lib:

You don't have to. The warning is just that: a warning. No error occurs when you see it, and it is not an indication of the program doing the wrong thing. It's entirely expected behaviour, and this is working as designed. However, if you're concerned about them, you may call close.

and

The reason the resource warnings are back is because we're much better about not returning partially complete connections to the connection pool. The lack of warnings in 2.5.4 was actually symptomatic of a problem: weirdly, the warnings are an indication that everything is working as intended.

So looks like not an issue. Why is it coming up though? It looks like Python is supposed to suppress such warnings by default. Seems like somewhere around 3.2 (?) they changed the behavior of unittest to override the default behavior on warnings. You can suppress the warnings following this SO. This makes it clear why, if you google around for this issue, it seems to occur a lot in the context of "I did a unit test and now this!".

(* edit, added credit where credit is due)

@jdufresne
Copy link
Contributor

You don't have to. The warning is just that: a warning. No error occurs when you see it, and it is not an indication of the program doing the wrong thing. It's entirely expected behaviour, and this is working as designed. However, if you're concerned about them, you may call close.

I find this advice contrary to Python's typical approach to resource handling. Python encourages deterministic resource handling. For example, the stdlib produces warnings when files are mismanaged in a non-deterministic way to help guide developers. Developers should take action on the warnings produced by stdlib. See the following Python bug where core developers discuss warnings during destructors:

https://bugs.python.org/issue28867

Additionally, Python docs warn against relying on __del__ for resource management:

It is not guaranteed that __del__() methods are called for objects that still exist when the interpreter exits.

And, exceptions occurring during __del__ are ignored and can't be handled:

Due to the precarious circumstances under which del() methods are invoked, exceptions that occur during their execution are ignored, and a warning is printed to sys.stderr instead.

I run all my tests and environments with warnings enabled to help catch real bugs and upgrade paths. If this warning really should be ignored as suggested here, then maybe it shouldn't be logged. As it is now, people are taking this advice of not calling close and this results in lots of (ignorable?) noise in my output.

@Naddiseo
Copy link

Another work around based upon boto/botocore#1231 is using the event system:

def http_closer(http_response, **kwargs):
    if http_response:
        http_response.close()

s3 = boto3.resource('s3', aws_access_key_id="XXX", aws_secret_access_key="YYY")
events = s3.meta.client.meta.events
# For HeadBucket for example, 'after-call.s3.*' could also work
events.register("after-call.s3.HeadBucket", http_closer)

try:
    s3.meta.client.head_bucket(Bucket="mybucket")
except botocore.exceptions.ClientError as e:
    print("Could not find bucket", e)

@scchess
Copy link

scchess commented Jan 7, 2018

So no fix? Warning is still important. boto3 developers are lazy.

@nchammas
Copy link

nchammas commented Jan 7, 2018

@student-t

So no fix? Warning is still important. boto3 developers are lazy.

Don’t use this kind of language, especially not on an open source project where the developers owe you nothing and you are getting their work for free.

If this issue is so important to you that you can’t wait for a fix to be submitted and merged in, then fork the project and fix it yourself, and spare us all this kind of low quality whining.

@lmeraz
Copy link

lmeraz commented Feb 3, 2018

I was receiving the same warning when working with Polly. The following implementation removed the warning. Running python=3.6, Boto3=1.5.

# core.py
import boto3
from botocore.exceptions import BotoCoreError
from contextlib import closing

polly = boto3.client('polly')

def polly_stream(text):
    try:
        response = polly.synthesize_speech(
            OutputFormat='mp3',
            Text=text,
            TextType='text',
            VoiceId='Salli',)
        with closing(response["AudioStream"]) as stream:
            return stream.read()
    except BotoCoreError as error:
        raise error
# test.py
from unittest import main, TestCase
from core import polly_stream


class TestVoice(TestCase):

    def test_polly_stream(self):
        stream = polly_stream('I')
        self.assertIsInstance(stream, bytes)


if __name__ == '__main__':
     main()
Ran 1 test in 0.229s

OK

Process finished with exit code 0

Hope it helps.

@wimglenn
Copy link

Same issue for me. If the warning can be safely ignored, please squash it in lib. I don't want to disable all ResourceWarning.

Python 3.6.4 on Linux
boto3==1.6.19
botocore==1.9.19

@jannesh
Copy link

jannesh commented Apr 12, 2018

Having the same issue. The other fixes did not work for me, but this one did. Added the following to my setUp:

def setUp(self):
    warnings.filterwarnings("ignore", category=ResourceWarning, message="unclosed.*<ssl.SSLSocket.*>") 

The message parameter is a regex match for the message part of the warning, so this can be adjusted to filter out only this one warning and leave the others intact.

Python: 3.6.2 on Windows
boto3: 1.4.4
botocore:1.5.84

@kiefersmith
Copy link

Bumping.

@kdaily kdaily added feature-request This issue requests a feature. needs-review and removed needs-discussion labels Aug 18, 2021
@kdaily
Copy link
Member

kdaily commented Aug 18, 2021

Hi all, I'm reviewing this with our team.

@swirle13
Copy link

@kdaily What's the status after reviewing this with the team?

@sunnova-mattbest
Copy link

I would also like to see some sort of .close() or similar teardown function exposed to eliminate these warnings. Thank you.

@hannes-ucsc
Copy link

hannes-ucsc commented Jan 25, 2022

I wouldn't come to that conclusion. It may be difficult to fix without a supporting change in CPython.

Edit: I was referring to a now deleted comment.

@kdaily
Copy link
Member

kdaily commented Jan 25, 2022

@davissclark,

I understand your frustration. Your previous comment has been removed as a violation of the Code of Conduct. Please take a moment to review it and keep the conversation productive. Thank you.

@boto boto deleted a comment from davissclark Jan 25, 2022
@ExplodingCabbage
Copy link

ExplodingCabbage commented Jan 26, 2022

I'll avoid reposting the exact words to avoid the wrath of the censors*, but just to avoid the deletion rendering @hannes-ucsc's reply incoherent to future readers, suffice it to say that the deleted comment tersely suggested that since this bug has not been fixed in 5 years it is clear that the maintainers do not consider it worth their time and will not fix it.

* (well, hopefully, at least. @kdaily gives no indication of what precisely was deemed a CoC violation so we have to guess. Perhaps it was the meaning and not the phrasing that was deemed objectionable and I'm about to get CoCed too. 🤷)

@kellertk
Copy link
Contributor

kellertk commented Jan 26, 2022

Hi @ExplodingCabbage - I'm Tom and I manage the Developer Support team for the AWS SDKs (that is, the team that is responsible for managing our GitHub issues for the SDKs). It's well-taken feedback that sometimes moderation decisions can lead to other readers wondering what happened. This isn't uncommon in Internet communities, but I've asked the team to try and be more descriptive when possible if we do have to take moderation action on a comment. There may be reasons why we're unable to share why we reached a decision on a moderation action but that should be the exception rather than the rule.

@kdaily already linked our Code of Conduct, but to answer your specific question we decided to take action because we felt this comment did not "use welcoming and inclusive language" and could "reasonably be considered inappropriate in a professional setting".

The best way to let us know an issue is important to you is by reacting to it with a 👍, ❤️, 🚀, or 👀. We have internal tooling that sorts feature request issues by these comment reactions and use that data to plan future work. We also use other reactions, number of comments, and other data to drive those decisions, but the reactions are one of the most important.

About this issue: I know the team is planning on addressing it soon. We should have replied earlier with a status update, but your feedback does mean that this is important to us. It has fallen down in the priority list because it appears that this is a nuisance warning, but the community feedback means we're still watching.

@ExplodingCabbage
Copy link

@kellertk Yeah, I figured the language used was probably the reason. Without it being explicitly stated, one can never be sure, though!

@nateprewitt
Copy link
Contributor

Hi everyone,

We completely understand the annoyance with having the warning in your logs. To be clear, there are no performance implications by this warning, it's a loud misnomer. Many of you likely don't care about the cause and are looking for a way to reduce the noise though. Most of the relevant information is now buried in this thread, so I'll provide a brief overview of the issue and then we can talk solutions.

Overview

We're hesitant to handle this warning in botocore itself. We don't have a clear indicator when the warning is actually valid and would risk unintentionally masking real problems. As @hannes-ucsc recently noted, they provided a very useful explanation of the underlying issue in CPython. urllib3 keeps connections alive in a pool for performance reasons, however, CPython doesn't recognize this and warns that they haven't been appropriately cleaned up. Removing these warnings entirely would require a change at that level.

Solutions

There have been a few notable proposals in boto/botocore#1231 and boto/botocore#1810. Both have some concerns that aren't easy to address which I'll highlight below.

boto/botocore#1231 presents a solution of trying to close on every call. The problem here is urllib3 is returning a ConnectionPool, not a Connection, despite the confusing naming. That means we're now tearing down our entire pool on each call which is going to have a non-trivial performance hit for the average user. This makes a change like this a non-starter.

boto/botocore#1810 has some surprising side effects that are going to catch people. When the user calls close, it invokes it on the PoolManager. That not only closes the connections for the client/resource in use, but ALL clients and resources created by the underlying Session. This may lead to unexpected failures in parallelized environments and introduces a new level of management required for client cleanup. You MUST ensure all users of the Session are ready for teardown before invoking close.

Of the two proposals, boto/botocore#1810 is the more acceptable given the amount of demand. We do need to make sure the implications are clearly documented for users though. We'll start looking at driving a conclusion on boto/botocore#1810 that will enable managing this manually.

@lukasschmit
Copy link

bumping!

@nateprewitt
Copy link
Contributor

Hi @lukasschmit, thanks for checking in, we're still aware of the request. Generally, posting bumps isn't helpful in GitHub issues because it buries relevant responses. We're actively tracking the work but it's blocked behind other recent changes. We'll post an update here when it's ready to be merged.

@nateprewitt
Copy link
Contributor

nateprewitt commented Jun 9, 2022

Alright, I've fixed the remaining blockers and the close() change should be ready to release. We've merged boto/botocore#1810 which will be available in the next version of boto3.

Users can force connection cleanup by either calling close() directly:

client = boto3.client(service_name='s3')
buckets = client.list_buckets()
client.close()

or

Wrap your client with the closing context manager:

from contextmanager import closing
with closing(boto3.client(service_name='s3')) as client:
    buckets = client.list_buckets()

Again we'll reiterate that the warnings themselves are not an issue. It's an issue in the standard library incorrectly warning because urllib3 keeps connections alive in our ConnectionPool. They are managed and will be reclaimed when appropriate. For users who want to avoid the warnings, but not suppress them with a filter, close() is now available on the client to clear the pool.

Note that calling close() routinely will have performance implications if the client is still in use. It should only be performed when the client is no longer needed.

@github-actions
Copy link

github-actions bot commented Jun 9, 2022

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@wimglenn
Copy link

wimglenn commented Jun 9, 2022

For the record, @nateprewitt means boto/botocore#1810 (in botocore) not the link in boto3

@nateprewitt
Copy link
Contributor

Great note @wimglenn, I'll update the original. Thanks!

@rdar-lab
Copy link

rdar-lab commented Dec 26, 2022

For python 3.10:

from contextlib import closing
with closing(boto3.client(service_name='s3')) as client:
     buckets = client.list_buckets()

@rickerish-nah
Copy link

Issue: ResourceWarning: unclosed ssl.SSLSocket

is it possible to use the s3 client as/within a context manager? this way we can resolve the close() issue for respective usage specifically but not close down on all other parallel sessions or decrease in performance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request This issue requests a feature. needs-review
Projects
None yet
Development

Successfully merging a pull request may close this issue.