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.

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

Issue 408 fix, Memory leak was generated from Event #524

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Issue 408 fix, Memory leak was generated from Event #524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
315c277
#408, fix for MemLeak on event exceptions
kashirin-alex Sep 11, 2018
53f00cb
#408, Test for fix for MemLeak on event exceptions
kashirin-alex Sep 12, 2018
4e293eb
#408, fix for MemLeak on event exceptions
kashirin-alex Sep 12, 2018
361210d
#408, fix for MemLeak on event exceptions …
kashirin-alex Sep 12, 2018
99350ca
Test for fix for MemLeak on event exceptions …
kashirin-alex Sep 12, 2018
9dce8c2
#408, fix for MemLeak on event exceptions
kashirin-alex Sep 12, 2018
454a897
#408, fix for MemLeak on event exceptions
kashirin-alex Sep 12, 2018
0291f64
Merge branch 'issue-408' of https://github.com/kashirin-alex/eventlet…
kashirin-alex Sep 12, 2018
3a59e79
#408, fix for MemLeak on event exceptions
kashirin-alex Sep 12, 2018
1b51236
#408, fix for MemLeak on event exceptions
kashirin-alex Sep 12, 2018
9888aa6
#408, fix for MemLeak on event exceptions
kashirin-alex Sep 12, 2018
1d4cc2a
event_test, clear threads not del
kashirin-alex Sep 12, 2018
98e07fd
event_test, version by sys.version_info.major and count obj refs prio…
kashirin-alex Sep 12, 2018
e0c12b6
event_test, version by sys.version_info.major and count obj refs prio…
kashirin-alex Sep 12, 2018
a2c8e6d
event_test, version by sys.version_info.major and count obj refs prio…
kashirin-alex Sep 12, 2018
b985825
event_test, version by sys.version_info.major and count obj refs prio…
kashirin-alex Sep 12, 2018
a55b2e9
event_test, some counting info with assert
kashirin-alex Sep 12, 2018
0ee4c90
event_test, some counting info with assert
kashirin-alex Sep 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions eventlet/event.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ class Event(object):
>>> evt.wait()
4
"""
_result = None
_exc = None

def __init__(self):
self._result = None
self._exc = None
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems unrelated to the issue at hand.

Copy link
Author

Choose a reason for hiding this comment

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

At this point it is the only thing that can cause a one not garbage collected object.

self._waiters = set()
self.reset()

Expand Down
2 changes: 1 addition & 1 deletion eventlet/greenthread.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ class GreenThread(greenlet.greenlet):
"""

def __init__(self, parent):
greenlet.greenlet.__init__(self, self.main, parent)
super(GreenThread, self).__init__(self.main, parent)
Copy link
Contributor

Choose a reason for hiding this comment

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

Also seems unrelated to the issue at hand

self._exit_event = event.Event()
self._resolving_links = False
self._exit_funcs = None
Expand Down
43 changes: 43 additions & 0 deletions tests/event_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,3 +103,46 @@ def test_wait_timeout_exceed():
td = eventlet.hubs.get_hub().clock() - t1
assert not result
assert td >= delay


def test_no_mem_leaks():
jstasiak marked this conversation as resolved.
Show resolved Hide resolved

import objgraph
import gc
import sys
Copy link
Contributor

Choose a reason for hiding this comment

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

Those can be imported at the module level just fine, why import here?

eventlet.hubs.get_hub().set_timer_exceptions(False)

ref_c_foo = objgraph.count('Foo')
ref_c_gt = objgraph.count('eventlet.greenthread.GreenThread')

threads = {}

class Foo(object):
def __init__(self, idx):
self.thread = threads[idx]

def target(idx):
foo = Foo(idx)
if idx % 2 == 0:
raise Exception("boom")

for index in range(10):
gt = eventlet.spawn(target, index)
threads[index] = gt

eventlet.sleep()
threads.clear()
while gc.collect():
pass

eventlet.hubs.get_hub().set_timer_exceptions(True)
Copy link
Contributor

Choose a reason for hiding this comment

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

We have to make sure this is executed even if this test fails earlier on.


ref_c_gt += 1
if sys.version_info.major == 2:
ref_c_foo += 1
ref_c_gt += 1

c_foo = objgraph.count('Foo')
c_gt = objgraph.count('eventlet.greenthread.GreenThread')
assert c_foo == ref_c_foo, 'expected: ' + str(ref_c_foo) + ' counted: ' + str(c_foo)
assert c_gt == ref_c_gt, 'expected: ' + str(ref_c_gt) + ' counted: ' + str(c_gt)
1 change: 1 addition & 0 deletions tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ deps =
py{27,34}-{selects,poll,epolls}: pyopenssl==17.3.0
setuptools==38.5.1
{selects,poll,epolls}: pyzmq==17.0.0
objgraph==3.4.0
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an unnecessarily heavy dependency for what's being done here, we can count the amount of objects of a specific type just fine without it.

Copy link
Author

Choose a reason for hiding this comment

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

Will be great to know which method can replace the objgraph.count('eventlet.greenthread.GreenThread')

Copy link
Contributor

Choose a reason for hiding this comment

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

>>> import gc
>>> import objgraph
>>> 
>>> class X:
  2     pass
>>> 
>>> objgraph.count('X')
0
>>> sum(1 for o in gc.get_objects() if isinstance(o, X))
0
>>> x = X()
>>> objgraph.count('X')
1
>>> sum(1 for o in gc.get_objects() if isinstance(o, X))
1

Copy link
Author

Choose a reason for hiding this comment

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

OK, thanks,
I'll keep it updated with a solution to overcome/change the use of self._exc (if found of-course)

Copy link
Author

Choose a reason for hiding this comment

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

it might should be a method of the tests/init.py, and it has already gc imported

def check_ref_count(Instance, expected=0):
    c = sum(1 for o in gc.get_objects() if isinstance(o, Instance))
    assert c == expected, \
        "%s ref expected: %d counted: %d " % (Instance, expected, c)

Copy link
Author

Choose a reason for hiding this comment

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

or it can be better with a with statement

def check_ref_count(Instance, expected=0):
    try:
        pre_c = sum(1 for o in gc.get_objects() if isinstance(o, Instance))    
    finally:
        c = sum(1 for o in gc.get_objects() if isinstance(o, Instance))
        assert c-pre_c == expected, \
            "%s ref pre-counted: %d expected: %d \ counted: %d "
             % (Instance, pre_c, expected, c)

and work with:

with check_ref_count(Instance):
    #code

commands =
nosetests --verbose {env:tox_cover_args} {posargs:tests/}
coverage xml -i