Skip to content

bpo-35879: Fix type comment leaks #11728

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 1, 2019
Merged

bpo-35879: Fix type comment leaks #11728

merged 2 commits into from
Feb 1, 2019

Conversation

gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented Feb 1, 2019 •
edited by bedevere-bot
Loading

@gvanrossum
Copy link
Member Author

Since this just fixes a bug introduced by yesterday's merge of GH-11645, I don't think we need a blurb.

gvanrossum pushed a commit to python/typed_ast that referenced this pull request Feb 1, 2019
Fix memory leaks caused by type comment parsing
6f47b31 
Echoes python/cpython#11728 (they were found
by the CPython refleak detector, see
https://bugs.python.org/issue35879).
pablogsal
pablogsal approved these changes Feb 1, 2019
View reviewed changes
Copy link
Member

@pablogsal pablogsal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Finally! I spent al leat 3 hours tracking this leak. How did you find that PyArena_AddPyObject was the missing piece?

@pablogsal
Copy link
Member

I have run the entire test suite with ref leaks activated and it succeeds. Here is test_type_comments:

~/github/cpython remotes/upstream/fix-type-comment-leaks ✔
❯ ./python -m test test_type_comments -R 10:10
Run tests sequentially
0:00:00 load avg: 0.59 [1/1] test_type_comments
beginning 20 repetitions
12345678901234567890
....................

== Tests result: SUCCESS ==

1 test OK.

Total duration: 168 ms
Tests result: SUCCESS

@gvanrossum
Copy link
Member Author

How did you find that PyArena_AddPyObject was the missing piece?

I kept reading the source, trying to understand how this was done for other fields. Finally I found that identifiers had the same pattern (a string == PyObject * in the struct) and then realized that those are allocated via new_identifier()/NEW_IDENTIFIER(), which was a pattern very similar to new_type_comment()/NEW_TYPE_COMMENT(), and reading through it found that call at the end. This also solved the mystery why new_type_comment() had an unused struct compiling *c argument that was passed by the NEW_TYPE_COMMENT() macro -- this was copied NEW_IDENTIFIER(), where it was needed for the arena.

@gvanrossum gvanrossum merged commit d2b4c19 into master Feb 1, 2019
@gvanrossum gvanrossum deleted the fix-type-comment-leaks branch February 1, 2019 23:28
@gvanrossum gvanrossum mentioned this pull request Feb 1, 2019
Merged
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot PPC64 Fedora 3.x has failed when building commit d2b4c19.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/53/builds/2271) and take a look at the build logs.
  4. Check if the failure is related to this commit (d2b4c19) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/53/builds/2271

Click to see traceback logs 
Traceback (most recent call last):
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/pool.py", line 129, in worker
    put((job, i, result))
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/queues.py", line 368, in put
    self._writer.send_bytes(obj)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/connection.py", line 200, in send_bytes
    self._send_bytes(m[offset:offset + size])
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/connection.py", line 411, in _send_bytes
    self._send(header + buf)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/connection.py", line 368, in _send
    n = write(self._handle, buf)
BrokenPipeError: [Errno 32] Broken pipe

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/process.py", line 302, in _bootstrap
    self.run()
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/process.py", line 99, in run
    self._target(*self._args, **self._kwargs)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/pool.py", line 134, in worker
    put((job, i, (False, wrapped)))
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/queues.py", line 368, in put
    self._writer.send_bytes(obj)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/connection.py", line 200, in send_bytes
    self._send_bytes(m[offset:offset + size])
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/connection.py", line 411, in _send_bytes
    self._send(header + buf)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/connection.py", line 368, in _send
    n = write(self._handle, buf)
BrokenPipeError: [Errno 32] Broken pipe
Process SpawnPoolWorker-229:
Traceback (most recent call last):
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/pool.py", line 129, in worker
    put((job, i, result))
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/queues.py", line 368, in put
    self._writer.send_bytes(obj)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/connection.py", line 200, in send_bytes
    self._send_bytes(m[offset:offset + size])
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/connection.py", line 411, in _send_bytes
    self._send(header + buf)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/connection.py", line 368, in _send
    n = write(self._handle, buf)
BrokenPipeError: [Errno 32] Broken pipe

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/process.py", line 302, in _bootstrap
    self.run()
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/process.py", line 99, in run
    self._target(*self._args, **self._kwargs)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/pool.py", line 134, in worker
    put((job, i, (False, wrapped)))
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/queues.py", line 368, in put
    self._writer.send_bytes(obj)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/connection.py", line 200, in send_bytes
    self._send_bytes(m[offset:offset + size])
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/connection.py", line 411, in _send_bytes
    self._send(header + buf)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/connection.py", line 368, in _send
    n = write(self._handle, buf)
BrokenPipeError: [Errno 32] Broken pipe
Process SpawnPoolWorker-230:
Traceback (most recent call last):
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/pool.py", line 129, in worker
    put((job, i, result))
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/queues.py", line 368, in put
    self._writer.send_bytes(obj)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/connection.py", line 200, in send_bytes
    self._send_bytes(m[offset:offset + size])
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/connection.py", line 411, in _send_bytes
    self._send(header + buf)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/connection.py", line 368, in _send
    n = write(self._handle, buf)
BrokenPipeError: [Errno 32] Broken pipe

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/process.py", line 302, in _bootstrap
    self.run()
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/process.py", line 99, in run
    self._target(*self._args, **self._kwargs)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/pool.py", line 134, in worker
    put((job, i, (False, wrapped)))
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/queues.py", line 368, in put
    self._writer.send_bytes(obj)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/connection.py", line 200, in send_bytes
    self._send_bytes(m[offset:offset + size])
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/connection.py", line 411, in _send_bytes
    self._send(header + buf)
  File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/connection.py", line 368, in _send
    n = write(self._handle, buf)
BrokenPipeError: [Errno 32] Broken pipe
/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/multiprocessing/semaphore_tracker.py:164: UserWarning: semaphore_tracker: There appear to be 4 leaked semaphores to clean up at shutdown
  warnings.warn('semaphore_tracker: There appear to be %d '

@pablogsal
Copy link
Member

This failure is not related with this PR (is some broken pipe in multiprocessing).

gvanrossum pushed a commit to python/typed_ast that referenced this pull request Feb 5, 2019
Fix memory leaks caused by type comment parsing
b7d582a 
Echoes python/cpython#11728 (they were found
by the CPython refleak detector, see
https://bugs.python.org/issue35879).
@gvanrossum gvanrossum mentioned this pull request Feb 5, 2019
Merged
gvanrossum added a commit to python/typed_ast that referenced this pull request Feb 6, 2019
@gvanrossum
Fix memory leaks caused by type comment parsing (#91)
c6d7fcb 
Echoes python/cpython#11728 (they were found
by the CPython refleak detector, see
https://bugs.python.org/issue35879).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pablogsal pablogsal pablogsal approved these changes

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gvanrossum @pablogsal @bedevere-bot @the-knights-who-say-ni