Closed Bug 1139721 Opened 5 years ago Closed 4 years ago

[B2G][Camera][Gecko] Fix memory leaks uncovered by existing automated tests

Categories

(Firefox OS Graveyard :: Gaia::Camera, defect)

x86_64
Linux
defect
Not set

Tracking

(firefox40 fixed)

RESOLVED FIXED
2.2 S10 (17apr)
Tracking Status
firefox40 --- fixed

People

(Reporter: aosmond, Assigned: aosmond)

References

Details

Attachments

(1 file, 2 obsolete files)

The following leaks are already caught by the existing unit tests, but the leak check output has gone unnoticed until now:

1) When the camera fails to initialize and the application's CameraManager.getCamera call fails, we never supply a reference to the CameraControl object so the application can not call release. However the only way we presently free the GonkCameraControl object is if CameraControl.release is called. We need to free it automatically.

2) CameraCapabilities does not call MOZ_COUNT_CTOR but does call MOZ_COUNT_DTOR resulting in a bad leak summary.

3) When CameraControl's onfacesdetected event is generated, we add an extra unnecessary reference to each DOMCameraDetectedFace object. Not only do those objects leak, they hold a reference to CameraControl as well which causes many other objects to be held onto.
Attached patch bug1139721.patch, v1 (obsolete) — Splinter Review
Assignee: nobody → aosmond
Status: NEW → ASSIGNED
Flags: in-testsuite+
Attachment #8572972 - Flags: review?(mhabicher)
Comment on attachment 8572972 [details] [diff] [review]
bug1139721.patch, v1

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6cfc61a29c3e

Looks like I might have missed something else since the debug emulator crashed in one of our tests but the optimized emulator passed.
Attachment #8572972 - Flags: review?(mhabicher)
Depends on: 1121855
Comment on attachment 8572972 [details] [diff] [review]
bug1139721.patch, v1

Review of attachment 8572972 [details] [diff] [review]:
-----------------------------------------------------------------

Good work!
Attachment #8572972 - Flags: review+
Blocks: 1139027
No longer blocks: 1139027
Fix typo in comment.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=270194578fb6
Attachment #8572972 - Attachment is obsolete: true
Attachment #8577296 - Flags: review+
Keywords: checkin-needed
Backed out in https://hg.mozilla.org/integration/b2g-inbound/rev/355ac9495b03 for being the likely cause of frequent b2g debug mochitest-5 failures:

https://treeherder.mozilla.org/logviewer.html#?job_id=1521747&repo=b2g-inbound
Flags: needinfo?(aosmond)
Looks like the problem may have been solved or otherwise made unreproducible, because it doesn't happen when I run the camera tests now (in mochitest-6 grouping now), both on the try server and locally (50+ attempts overnight).

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4e8fce43dba6
Flags: needinfo?(aosmond)
Rebase.
Attachment #8577296 - Attachment is obsolete: true
Attachment #8593891 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/7a458d7e3e4f
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S10 (17apr)
You need to log in before you can comment on or make changes to this bug.