Skip to content

"s*" argument parser marker leaks memory #47918

Closed
Closed
"s*" argument parser marker leaks memory#47918
Assignees
pitrou
Labels
performancePerformance or resource usagerelease-blocker

Description

@amauryfa
amauryfa
opened on Aug 24, 2008
BPO 3668
Nosy @amauryfa, @pitrou
Files
  • argleak.patch
  • argleak2.patch
  • argleak-2.6.patch
  • argleak3.patch
  • argleak2-2.6.patch
    		•

    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/pitrou'
closed_at = <Date 2008-08-29.18:42:00.425>
created_at = <Date 2008-08-24.21:02:57.594>
labels = ['release-blocker', 'performance']
title = '"s*" argument parser marker leaks memory'
updated_at = <Date 2008-08-29.18:42:00.424>
user = 'https://github.com/amauryfa'

    bugs.python.org fields:

    activity = <Date 2008-08-29.18:42:00.424>
actor = 'pitrou'
assignee = 'pitrou'
closed = True
closed_date = <Date 2008-08-29.18:42:00.425>
closer = 'pitrou'
components = []
creation = <Date 2008-08-24.21:02:57.594>
creator = 'amaury.forgeotdarc'
dependencies = []
files = ['11243', '11244', '11245', '11246', '11248']
hgrepos = []
issue_num = 3668
keywords = ['patch', 'needs review']
message_count = 12.0
messages = ['71870', '71917', '71918', '71923', '71924', '71925', '71926', '71927', '71929', '72123', '72126', '72149']
nosy_count = 2.0
nosy_names = ['amaury.forgeotdarc', 'pitrou']
pr_nums = []
priority = 'release blocker'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue3668'
versions = ['Python 2.6', 'Python 3.0']

    Activity

    1. amauryfa commented on Aug 24, 2008

      @amauryfa
      amauryfa
      on Aug 24, 2008
      ContributorAuthor

      When PyArg_ParseTuple correctly parses a s* format, but raises an
      exception afterwards (for a subsequent parameter), the user code will
      not call PyBuffer_Release() and memory will leak.
      Seen by "regrtest -R:: test_binascii"

      For example:
>>> binascii.a2b_qp("", **{1:1})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: keywords must be strings
[42278 refs]
>>> binascii.a2b_qp("", **{1:1})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: keywords must be strings
[42279 refs]
>>> binascii.a2b_qp("", **{1:1})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: keywords must be strings
[42280 refs]

      The same pattern was correctly handled by the "et#" type (where the user
      has to call PyMem_Free) with the help of a cleanup list (see the
      addcleanup() function in getargs.c). (See bpo-501716)

    2. amauryfa
      added
      release-blocker
      on Aug 24, 2008
    3. pitrou
      self-assigned this
      on Aug 25, 2008

    4. pitrou commented on Aug 25, 2008

      @pitrou
      pitrou
      on Aug 25, 2008
      Member

      Here is a patch. Please review.

    5. pitrou
      added
      performancePerformance or resource usage
      on Aug 25, 2008

    6. amauryfa commented on Aug 25, 2008

      @amauryfa
      amauryfa
      on Aug 25, 2008
      ContributorAuthor

      This patch elegantly reuses the existing cleanup list.

      Only two remarks:

      • there are a few tabs/spaces inconsistencies.
      • I would make the cleanup_ptr explicit:
        intead of
        addcleanup(*buffer, freelist, NULL);
        I'd prefer
        addcleanup(*buffer, freelist, cleanup_ptr);

    7. pitrou commented on Aug 25, 2008

      @pitrou
      pitrou
      on Aug 25, 2008
      Member

      Ok, here is a new patch addressing Amaury's comments.

    8. pitrou commented on Aug 25, 2008

      @pitrou
      pitrou
      on Aug 25, 2008
      Member

      By the way, this bug affects 2.6 as well, although not in binascii since
      it has not been converted to use "s*":

      >>> codecs.latin_1_decode(b"", 0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: latin_1_decode() argument 2 must be string or None, not int
[57425 refs]
>>> codecs.latin_1_decode(b"", 0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: latin_1_decode() argument 2 must be string or None, not int
[57426 refs]

    9. pitrou commented on Aug 25, 2008

      @pitrou
      pitrou
      on Aug 25, 2008
      Member

      Here is a patch for 2.6.

    10. amauryfa commented on Aug 25, 2008

      @amauryfa
      amauryfa
      on Aug 25, 2008
      ContributorAuthor

      Both patches look good to me.

    11. pitrou commented on Aug 25, 2008

      @pitrou
      pitrou
      on Aug 25, 2008
      Member

      Actually, here is a better patch for py3k.

    12. pitrou commented on Aug 25, 2008

      @pitrou
      pitrou
      on Aug 25, 2008
      Member

      And a similarly better patch for 2.6.

    13. pitrou commented on Aug 29, 2008

      @pitrou
      pitrou
      on Aug 29, 2008
      Member

      Amaury, are these patches ok to check in?

    14. amauryfa commented on Aug 29, 2008

      @amauryfa
      amauryfa
      on Aug 29, 2008
      ContributorAuthor

      Yes, let them go in!

    15. pitrou commented on Aug 29, 2008

      @pitrou
      pitrou
      on Aug 29, 2008
      Member

      Committed in r66057 and r66058.

    16. pitrou
      closed this as completedon Aug 29, 2008
    17. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    performancePerformance or resource usagerelease-blocker

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions