Skip to content

bpo-35879: Fix type comment leaks#11728

Merged
gvanrossum merged 2 commits intomasterfrom
fix-type-comment-leaks
Feb 1, 2019
Merged

bpo-35879: Fix type comment leaks#11728
gvanrossum merged 2 commits intomasterfrom
fix-type-comment-leaks

Conversation

@gvanrossum
Copy link
Member

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

@gvanrossum
Copy link
Member Author

gvanrossum commented Feb 1, 2019

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

pablogsal commented Feb 1, 2019

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

gvanrossum commented Feb 1, 2019

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

bedevere-bot commented Feb 2, 2019

⚠️⚠️⚠️ 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

pablogsal commented Feb 2, 2019

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