Skip to content

Ignored ResourceWarning warnings leak memory in warnings registries #71722

Closed

Description

@ghost
BPO 27535
Nosy @rhettinger, @ncoghlan, @vstinner, @bitdancer, @serhiy-storchaka, @thehesiod, @AraHaan
PRs
  • bpo-27535: Fix memory leak with warnings ignore #4489
  • [WIP] bpo-27535: Optimize warnings #4502
  • bpo-27535: Optimize warnings.warn() #4508
  • bpo-27535: Cleanup create_filter() #4516
  • [3.6] bpo-27535: Fix memory leak with warnings ignore (GH-4489) #4587
  • [2.7] bpo-27535: Fix memory leak with warnings ignore #4588
  • Files
  • memory.py
  • memory2.py
  • bench_ignore_warn.py
  • bench_c_warn.patch
  • bench_ignore_warn_c.py
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/vstinner'
    closed_at = None
    created_at = <Date 2016-07-17.01:53:42.897>
    labels = ['3.7', 'expert-IO', 'performance']
    title = 'Ignored ResourceWarning warnings leak memory in warnings registries'
    updated_at = <Date 2018-05-23.21:26:16.382>
    user = None

    bugs.python.org fields:

    activity = <Date 2018-05-23.21:26:16.382>
    actor = 'vstinner'
    assignee = 'vstinner'
    closed = False
    closed_date = None
    closer = None
    components = ['IO']
    creation = <Date 2016-07-17.01:53:42.897>
    creator = '\xd0\x90\xd0\xbb\xd0\xb5\xd0\xba\xd1\x81\xd0\xb0\xd0\xbd\xd0\xb4\xd1\x80 \xd0\x9a\xd0\xb0\xd1\x80\xd0\xbf\xd0\xb8\xd0\xbd\xd1\x81\xd0\xba\xd0\xb8\xd0\xb9'
    dependencies = []
    files = ['43762', '47281', '47282', '47286', '47287']
    hgrepos = []
    issue_num = 27535
    keywords = ['patch']
    message_count = 39.0
    messages = ['270600', '270608', '270609', '270644', '270647', '270666', '306636', '306645', '306648', '306649', '306652', '306661', '306664', '306695', '306696', '306712', '306716', '306721', '306723', '306725', '306735', '306758', '306773', '306828', '306919', '306920', '306923', '307057', '307058', '307059', '307061', '307133', '307135', '307137', '307139', '307161', '307165', '317421', '317460']
    nosy_count = 9.0
    nosy_names = ['rhettinger', 'ncoghlan', 'vstinner', 'r.david.murray', 'serhiy.storchaka', 'thehesiod', 'Decorater', '\xd0\x90\xd0\xbb\xd0\xb5\xd0\xba\xd1\x81\xd0\xb0\xd0\xbd\xd0\xb4\xd1\x80 \xd0\x9a\xd0\xb0\xd1\x80\xd0\xbf\xd0\xb8\xd0\xbd\xd1\x81\xd0\xba\xd0\xb8\xd0\xb9', 'jbfzs']
    pr_nums = ['4489', '4502', '4508', '4516', '4587', '4588']
    priority = 'low'
    resolution = None
    stage = 'resolved'
    status = 'open'
    superseder = None
    type = 'resource usage'
    url = 'https://bugs.python.org/issue27535'
    versions = ['Python 3.7']

    Activity

    1. ghost commented on Jul 17, 2016

      @ghost
      Author

      Actually, this issue is related to the warning module. The test script creates a lot of files with different names and deletes them. On the first pass the scripts calls the f.close() method for every file. On the second it doesn't.

      As a result, on the first pass, the memory consumption is constant and is about 3.9 Mb for my environment. For the second pass, the memory consumption constantly grows up to 246 Mb for 1 million files. I.e. memory leak is about 254 bytes for every opened file.

      This happens because of warning about not closed files. The memory is consumed for storing all possible warning messages generated for different file names to prevent further warnings with the same messages. Which is not the case if we are always opening new files.

      While not calling f.close() **might** lead to memory issues, the warning which notifies about this **absolutely guaranteed** leads to memory issue. Although f.close() is highly recommended, this is still the issue for code which doesn't use it for some reasons and doesn't experience other problems with not closed files because files are actually always closed in IOBase.__del__.

      This issue was discovered in this report: python-pillow/Pillow#2019

      The best way to fix this is excluding file name from warning message. As a result, the message for every file will be the same and warnings registry will not grow. For now, the message looks like this:

      ./memory.py:23: ResourceWarning: unclosed file <_io.FileIO name='__999.tmp' mode='wb' closefd=True>
      open_files(number, close)

      It may looks like this:

      ./memory.py:23: ResourceWarning: unclosed file <FileIO mode='wb' closefd=True>
      open_files(number, close)

    2. Repository owner added
      performancePerformance or resource usage
      on Jul 17, 2016
    3. serhiy-storchaka commented on Jul 17, 2016

      @serhiy-storchaka
      Member

      The file name is helpful for determining the source of the leak. If you don't want to close files in your script, set an appropriate warning filter to silence this kind of warnings.

    4. ghost commented on Jul 17, 2016

      @ghost
      Author

      @serhiy.storchaka

      Any filters not solves the problem because warnings module SAVES EVERY WARNING MESSAGE for further duplication checks.

      Yes, the file name is helpful for determining the source of the POSSIBLE leak. While file name in error message IS SOURCE OF THE LEAK. When you aware about not closed files, you can log file names. You have a way to do that. But if you want to ignore the warning, the memory will leak. Not because of not closed files, but because of warning itself.

      I do want to close files in my script, but in Pillow, it is not so easy due to backward compatibility.

    5. bitdancer commented on Jul 17, 2016

      @bitdancer
      Member

      I recommend rejecting this. Properly closing flies is the correct programming habit, which is why the ResourceWarning exists. The Pillow example seems to just indicate that people using Pillow need to pay attention to the ResourceWarning and close the files they open with pillow.

    6. serhiy-storchaka commented on Jul 17, 2016

      @serhiy-storchaka
      Member

      Thus, the problem is that even ignored warnings are saved. If there is a simple fix of this problem we can fix it. But if there is no simple and straightforward way, it may be not worth to complicate the code for such case. I'll look at the code.

    7. rhettinger commented on Jul 17, 2016

      @rhettinger
      Contributor

      I would like to see this fixed. It is rather unfortunate that a tool designed to help find possible resource leaks is itself a certain resource leak.

    8. jbfzs commented on Nov 21, 2017

      jbfzsmannequin
      Mannequin

      We just got hit by this. We had one specific case where files with unique names were not being closed and Python was leaking a lot of memory over a few days.

    9. vstinner commented on Nov 21, 2017

      @vstinner
      Member

      The problem is not the function displaying the warning, but warnings registries (warningregistry) no?

      The behaviour depends on the action of the filter matching the ResourceWarning warning:

      • always: don't touch the registry => no leak
      • ignore: add the key to registry => leak!
      • another action: add the key to registry => leak!

      The key is the tuple (text, category, lineno).

      I understand why always doesn't touch the registry. But why does "ignore" action touch the registry? Not only the user will not see the message, but the memory will slowly grow in the background.

    10. vstinner commented on Nov 21, 2017

      @vstinner
      Member

      I created PR 4489 to fix the memory leak for the "ignore "action". IMHO it's interesting to modify the "ignore" action because Python uses ignore many warnings by default:

      haypo@selma$ python3 -c 'import pprint, warnings; pprint.pprint(warnings.filters)'
      [('ignore', None, <class 'DeprecationWarning'>, None, 0),
      ('ignore', None, <class 'PendingDeprecationWarning'>, None, 0),
      ('ignore', None, <class 'ImportWarning'>, None, 0),
      ('ignore', None, <class 'BytesWarning'>, None, 0),
      ('ignore', None, <class 'ResourceWarning'>, None, 0)]

      I created memory2.py (based on attached memory.py) to measure the leak.

      Currently, the "always" action doesn't leak, but "ignore" and "default" actions leak:

      haypo@selma$ ./python -W always memory2.py
      Memory peak grow: +3.2 kB
      Warning filters:
      [('always',
      re.compile('', re.IGNORECASE),
      <class 'Warning'>,
      re.compile(''),
      0),
      ('ignore', None, <class 'BytesWarning'>, None, 0),
      ('always', None, <class 'ResourceWarning'>, None, 0)]

      haypo@selma$ ./python -W ignore memory2.py
      Memory peak grow: +26692.3 kB
      Warning filters:
      [('ignore',
      re.compile('', re.IGNORECASE),
      <class 'Warning'>,
      re.compile(''),
      0),
      ('ignore', None, <class 'BytesWarning'>, None, 0),
      ('always', None, <class 'ResourceWarning'>, None, 0)]

      haypo@selma$ ./python -W default memory2.py
      Memory peak grow: +26693.2 kB
      Warning filters:
      [('default',
      re.compile('', re.IGNORECASE),
      <class 'Warning'>,
      re.compile(''),
      0),
      ('ignore', None, <class 'BytesWarning'>, None, 0),
      ('always', None, <class 'ResourceWarning'>, None, 0)]

      With my PR 4489, the "ignore" action doesn't leak anymore:

      haypo@selma$ ./python -W ignore memory2.py
      Memory peak grow: +2.4 kB
      Warning filters:
      [('ignore',
      re.compile('', re.IGNORECASE),
      <class 'Warning'>,
      re.compile(''),
      0),
      ('ignore', None, <class 'BytesWarning'>, None, 0),
      ('always', None, <class 'ResourceWarning'>, None, 0)]

    11. jbfzs commented on Nov 21, 2017

      jbfzsmannequin
      Mannequin

      @vstinner Yes, from what I saw, the leak was from the registry / deduplication logic.

    12. changed the title [-]Memory leaks when opening tons of files[/-] [+]Ignored ResourceWarning warnings leak memory in warnings registries[/+] on Nov 21, 2017
    13. vstinner commented on Nov 21, 2017

      @vstinner
      Member

      The best way to fix this is excluding file name from warning message. As a result, the message for every file will be the same and warnings registry will not grow.

      Your warning comes from the io module which uses the message: "unclosed file %r" % file.

      I dislike the idea of removing the filename from this warning message, since the filename is very useful to identify where the bug comes from.

      Different things are discussed here:

      • First of all, if an application is correctly written, ResourceWarning is not emitted, and so Python doesn't leak memory

      • Using the "always" action, ResourceWarning doesn't leak neither.

      • Using a different action like "default", ResourceWarning does leak memory in hidden warning registries.

      While I don't see how we can avoid "leaking memory" (growing the registry) for actions like "once", I think that it's ok to don't touch the registry for the "ignore" action. So at least, an application ignoring ResourceWarning will not leak anymore.

      But it will still "leak" when you display ResourceWarning warnings with an action different than "always". In this case, IMHO the root issue is more the code which doesn't close the resource, than Python itself.

    14. 21 remaining items

    15. vstinner commented on Nov 27, 2017

      @vstinner
      Member

      I consider that PR 4489 is a bugfix, so I proposed backports to Python 2.7 (PR 4588) and 3.6 (PR 4587).

    16. vstinner commented on Nov 28, 2017

      @vstinner
      Member

      Victor: "I consider that PR 4489 is a bugfix, so I proposed backports to Python 2.7 (PR 4588) and 3.6 (PR 4587)."

      Serhiy: Are you ok to backport the change to stable branches?

    17. serhiy-storchaka commented on Nov 28, 2017

      @serhiy-storchaka
      Member

      I'm not sure. Ask the RM for 3.6.

      As for 2.7, does this problem exist in 2.7?

    18. vstinner commented on Nov 28, 2017

      @vstinner
      Member

      Serhiy: "As for 2.7, does this problem exist in 2.7?"

      Yes, see my PR 4588. If you run the test without the fix, the test fails.

    19. serhiy-storchaka commented on Nov 28, 2017

      @serhiy-storchaka
      Member

      I meant the original issue. AFAIK ResourceWarningis not raised in 2.7. And other warnings less likely have unique messages. Seems the problem is less critical in 2.7.

    20. vstinner commented on Nov 28, 2017

      @vstinner
      Member

      Serhiy: "I meant the original issue. AFAIK ResourceWarningis not raised in 2.7. And other warnings less likely have unique messages. Seems the problem is less critical in 2.7."

      Oh yes, sorry, I forgot your comment and I forgot that Python 2.7 doesn't have ResourceWarning. I closed my PR for Python 2.7: #4588

    21. vstinner commented on Nov 28, 2017

      @vstinner
      Member

      I ran a quick benchmark on Python 3.6: current code (3.6 branch, ref) => PR 4587 (patch):

      vstinner@apu$ python3 -m perf compare_to ref.json patch.json
      Mean +- std dev: [ref] 597 ns +- 10 ns -> [patch] 830 ns +- 15 ns: 1.39x slower (+39%)

      I don't want to backport an optimization to a stable branch, so I prefer to not backport the warnings change on the ignore action to Python 3.6. I rejected my PR 4587.

      The bug was fixed in master. I don't want to backport the change to 2.7 nor 3.6, so I close the issue.

    22. thehesiod commented on May 23, 2018

      thehesiodmannequin
      Mannequin

      not fixing this means that 3.6 slowly leaks for many people in prod. It's not often possible to fix all the warnings on large dynamic applications, I highly suggest finding a way to get this into 3.6. I bet there are a lot of frustrated people out there who don't know why their applications slowly leak.

    23. vstinner commented on May 23, 2018

      @vstinner
      Member

      Alexander Mohr reported a memory leak in his code using botocore (Python client for Amazon S3): bpo-33565. His code emited ResourceWarning, but these warnings are ignored by default. The link between ignored warnings and a memory leak is non obvious.

      Serhiy: what do you think of backporting my _warnings change for the ignore action?

    24. transferred this issue fromon Apr 10, 2022
    25. iritkatriel commented on Sep 12, 2022

      @iritkatriel
      Member

      Backport to 3.6 is no longer relevant.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions