Closed Bug 1165231 Opened 9 years ago Closed 9 years ago

Possible leaking in marionette runner

Categories

(Remote Protocol :: Marionette, defect)

defect
Not set
normal

Tracking

(firefox43 fixed)

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: pyang, Assigned: shinglyu)

References

Details

(Keywords: pi-marionette-runner)

Attachments

(1 file)

When runner is done, its object seems not to be collected by gc.

STR: 
1. run cli to create runner
2. set break point at place runner should be recycled
3. check memory usage by class type.
in ./client/marionette/runner/mixins/reporting.py

    self.mixin_run_tests.append(self.html_run_tests)
where self is runner, mixin_run_test is a list in runner, and html_run_test is runner's member function

That might create a cycle ref like

runner <- mixin_run_tests <- html_run_tests <- runner
Before: 

                                                 types |   # objects |   total size
====================================================== | =========== | ============
                                                   str |        4837 |    391.05 KB
                                                  list |          42 |     50.22 KB
                                                  dict |          32 |     27.88 KB
                                 <class 'random.Random |           1 |      4.95 KB
                                               unicode |          29 |      2.66 KB
                                                 tuple |          15 |      1.20 KB
                            builtin_function_or_method |          13 |    936     B
  <class 'marionette.marionette_test.MetaParameterized |           1 |    904     B
                                                  code |           5 |    640     B
                                               weakref |           6 |    528     B
                                                 float |          21 |    504     B
                                                   int |          14 |    336     B
         <class 'marionette.runner.base.MarionetteTest |           4 |    256     B
                                        instancemethod |           3 |    240     B
                                                   set |           1 |    232     B

After:
                                                 types |   # objects |   total size
====================================================== | =========== | ============
                                                   str |        4755 |    386.94 KB
                                                  list |          15 |     48.13 KB
                                                  dict |          11 |      5.63 KB
                                 <class 'random.Random |           1 |      4.95 KB
  <class 'marionette.marionette_test.MetaParameterized |           1 |    904     B
                                                 tuple |          10 |    864     B
                                               weakref |           6 |    528     B
                                                  code |           4 |    512     B
                                                   int |          10 |    240     B
                                                   set |           1 |    232     B
           <class 'mozlog.structuredlog.ComponentState |           3 |    192     B
                                                 float |           8 |    192     B
                                    function (test_ok) |           1 |    120     B
                                 function (test_isnot) |           1 |    120     B
                                    function (test_is) |           1 |    120     B

Notice the <class 'marionette.runner.base.MarionetteTest instances are freed
This looks like a great improvement to me!  Perhaps you can do a try run with the following syntax to make sure nothing is broken?

    -b o -p linux,linux64_gecko -u marionette,marionette-e10s,web-platform-tests-1,marionette-webapi,gaia-ui-test-functional,gaia-integration -t none
Flags: needinfo?(slyu)
Looks all green, thanks. https://treeherder.mozilla.org/#/jobs?repo=try&revision=26f8710f7adb
Flags: needinfo?(slyu) needinfo?(slyu) → needinfo?(ato) needinfo?(ato)
Comment on attachment 8655811 [details] [diff] [review]
bug-1165231-fix.patch

Even if you didn’t ask for my review, I’m going to go ahead and give this an r+.

Thank you!
Flags: needinfo?(ato)
Attachment #8655811 - Flags: review+
Thank you! I didn't ask anyone for review because I didn't know who to ask for review. Please don't take it personally. :)
Keywords: checkin-needed
(FWIW there’s a “suggested reviewer” drop down in Bugzilla that will give you an idea who to ask.)
https://hg.mozilla.org/mozilla-central/rev/ab1f3a15d424
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Assignee: nobody → slyu
See Also: → 1202529
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.