User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0 Build ID: 20141113030201 Steps to reproduce: 1. (Important) Launch Camera app and then back to homescreen 2. Launch Messages app 3. New message => attachment => camera 4. Take a picture => select 5. Back from New messages => discard 6. Repeat 3~5 steps Actual results: Memory usage of camera app is getting bigger continuously when checking via b2g-procrank. When repeated 30 times, the memory usage is Rss : 153528K, Pss : 136524K. Expected results: Memory usage of camera app should be stable.

This is reproduced in flame master. And I confirmed nsGlobalWindow for camera pick activity is not released even if the activity finished.

Hi, Kyle. Long time no see. I'm Youngwoo Jo. I met you in San Francisco office last year. Could you take this issue? I think this issue is related to cc/gc. So this issue seems to need to analyze cc/gc logs. I think you look like the best engineer for this issue. I'm preparing cc/gc logs for this issue.

This is a cc log extracted from flame v2.0 camera app process.

This is a gc log extracted from flame v2.0 camera app process.

Hi Youngwoo, I'm happy to help. Running the cycle collector scripts on the provided logs: $ python /c/dev/heapgraph/find_roots.py ~/Desktop/cc-edges.1230.1416849964.log 0xb3a29fb0 Parsing c:/Users/mozilla/Desktop/cc-edges.1230.1416849964.log. Done loading grap h. Reversing graph. Done. 0xb14e23d0 [DOMMediaStream] --[Preserved wrapper]--> 0xb0972840 [JS Object (CameraControl)] --[type_proto]--> 0xb1eb07f0 [JS Object (CameraControlPrototype)] --[getter]--> 0xb11b9800 [JS Object (Function - onAutoFocusMoving)] --[parent]--> 0xb1eaf0d0 [JS Object (Window)] --[define]--> 0xb09c85e0 [JS Object (Function - define)] --[fun_callscope]--> 0xb0998550 [JS Object (Call)] --[contexts]--> 0xb09b5790 [JS Object (Object)] --[_]--> 0xb09b1e70 [JS Object (Object)] --[deferreds]--> 0xb09b5940 [JS Object (Object)] --[BlobView]--> 0xb09f5580 [JS Object (Object)] --[promise]--> 0xb09f3f00 [JS Object (Object)] --[then]--> 0xb09f3f20 [JS Object (Function - prim/promise.then)] --[fun_callscope]--> 0xb09f4ba0 [JS Object (Call)] --[fail]--> 0xb09f4c40 [JS Object (Array)] --[objectElements[2]]--> 0xb1185e60 [JS Object (Function - finish)] --[parent]--> 0xb1185a00 [JS Object (Function - finish)] --[fun_callscope]--> 0xb09ccdc0 [JS Object (Call)] --[**UNKNOWN SLOT 0**]--> 0xb09ccd30 [JS Object (Call)] --[n]--> 0xb116b460 [JS Object (Function - newContext/makeErrback/<)] --[fun_callscope]--> 0xb09ccd00 [JS Object (Call)] --[d]--> 0xb09cc670 [JS Object (Object)] --[promise]--> 0xb113bf80 [JS Object (Object)] --[then]--> 0xb1140280 [JS Object (Function - prim/promise.then)] --[fun_callscope]--> 0xb09c9c40 [JS Object (Call)] --[fail]--> 0xb09c9ce0 [JS Object (Array)] --[objectElements[2]]--> 0xb0821860 [JS Object (Function - finish)] --[**UNKNOWN SLOT 2**]--> 0xb08216a0 [JS Object (Function - newContext/makeE rrback/<)] --[fun_callscope]--> 0xb09571f0 [JS Object (Call)] --[d]--> 0xb11daca0 [JS Object (Object)] --[factory]--> 0xb0821380 [JS Object (Function)] --[script]--> 0xb11a8f80 [JS Script] --[sourceObject]--> 0xb1ef0520 [JS Object (ScriptSource)] --[**UNKNOWN SLOT 1**]--> 0xb0972c60 [JS Object (HTMLScriptElement)] --[UnwrapDOMObject(obj)]--> 0xb2d8dac0 [FragmentOrElement (xhtml) script app ://camera.gaiamobile.org/index.html#pick] --[[via hash] mListenerManager]--> 0xb2b02940 [EventListenerManager] --[mListeners event=onload listenerType=3 [i]]--> 0xb134a7c0 [CallbackObject ] --[mIncumbentGlobal]--> 0xb3a29fb0 [nsGlobalWindow #13 inner app://camera.ga iamobile.org/index.html#pick] Root 0xb14e23d0 is a ref counted object with 1 unknown edge(s). known edges: 0xb0972840 [JS Object (CameraControl)] --[UnwrapDOMObject(obj)]--> 0xb14e23d0 So there's an unknown edge to the nsDOMCameraControl. From looking at the code it appears that nsDOMCameraControl calls mCameraControl->AddListener, but never calls RemoveListener. That would appear to set up a cycle that would explain this leak. Mike, can you take a look at this?

It seems that nsDOMCameraManager always new CameraControls by getting a new mWindowId every time runs Step 2 to launch the camera app. http://lxr.mozilla.org/mozilla-b2g32_v2_0/source/dom/camera/DOMCameraManager.cpp#379 Is it possible we check the hash table before |CameraControls* controls = sActiveWindows->Get(mWindowId)|? If nsDOMCameraManager contains camera control in hash table, we may reuse it to solve thie problem.

this is WP1 patch to remove listener on nsDOMCameraControl.

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #5) > So there's an unknown edge to the nsDOMCameraControl. From looking at the > code it appears that nsDOMCameraControl calls mCameraControl->AddListener, > but never calls RemoveListener. That would appear to set up a cycle that > would explain this leak. we verified kyle's advice. and we are uploading WP1 patch (attachment 8528387 [details] [diff] [review]) (Attachment 8528387 [details] [diff] is just only focused the purpose to remove listener on nsDOMCameraControl.) Due to never calls RemoveListener, it continuously increase memory because of stacking nsGlobalWindow which is created by camera's pick activity. and we don't know how to work nsDOMCameraControl, when DOMCameraControlListener removes, there are some edge case. and to fix edge case, we modified the calling of mCameraControl->Shutdown(). in finally, to fix this problem, we should remove DOMCameraControlListener when nsDOMCameraControl released. (http://dxr.mozilla.org/mozilla-central/source/dom/camera/DOMCameraControl.cpp#300) Mike, can you take a look at this?

Comment on attachment 8528387 [details] [diff] [review] Remove listener on CameraControl Review of attachment 8528387 [details] [diff] [review]: ----------------------------------------------------------------- this is an attempt to verify affect when it calls RemoveListener.

jeremy.kim.leo, why not just call mCameraControl->RemoveListener() at the end of the kHardwareClosed[1] case in OnHardwareStateChange()? 1. http://dxr.mozilla.org/mozilla-central/source/dom/camera/DOMCameraControl.cpp#1267 Also, when posting a patch, please make sure you include 8 lines of diff context: for |hg diff| and |git diff| this means specifying the -U8 option.

(In reply to Mike Habicher [:mikeh] from comment #10) > jeremy.kim.leo, why not just call mCameraControl->RemoveListener() at the > end of the kHardwareClosed[1] case in OnHardwareStateChange()? > > 1. http://dxr.mozilla.org/mozilla-central/source/dom/camera/DOMCameraControl. > cpp#1267 > this trial is failed. when it starts camera app, it always received CameraControlListener::kHardwareClosed at first of all. so, if we are just remove listener at here, it always don't have any listener after this. and can i ask you why it has only one mCameraThread for all DOMCameraControl? (http://dxr.mozilla.org/mozilla-central/source/dom/camera/CameraControlImpl.cpp#18)

I investigated more and more. because of comment 11, we need another way to remove listener. so, how about you like this way? let's clear listener on CameraControlImpl::Shutdown() and after doing this, When I try test, i found that the ~nsGonkCameraControl{StopImpl();} always called recursively and finally go to crash. So I changed the location to call stop() into CameraControlImpl::Shutdown(). Could you advice to me? Thank you

I've tried your patch with the following steps: 1. open Camera app 2. press and hold Home button to bring up the task switch (and with logcat open, wait for camera to stop) 3. tap on the Camera app window to reopen it 4. repeat steps 2 and 3. I don't see any destructors called on any of the iterations, so there's definitely a leak here. I will investigate.

Attachment 8529119 [details] [diff] didn't completely address this bug, since two other classes were also listening to ICameraControl, along with the per-window registry. This patch addresses those issues as well. tl;dr: 1. strong registry pointers replaced with weak references; 2. replaces all MOZ_ASSERT(!mCameraControl) instances with error reports; 3. makes CameraCapabilities and CameraRecorderProfiles listen for hardware-closed events, to clean up; 4. adds mAudioChannelAgent to nsDOMCameraControl's CYCLE_COLLECTION list; 5. removes some stable code from CameraControlImpl. With the above changes, I have confirmed that the cycle collector picks up stale instances of nsDOMCameraCOntrol. Usually 2 to 3 instances hang around, but they are eventually cleaned up as well. Will push to try once it reopens.

thanks mhabicher. it looks like the patch only based on master branch. we are focusing on v2.0 branch now. because "Dom/Camera" has huge difference between master and v2.0, it hard to apply on v2.0. Can you make the patch based on v2.0 also?

Comment on attachment 8529964 [details] [diff] [review] Fix memory leak in CameraControl, v1 Review of attachment 8529964 [details] [diff] [review]: ----------------------------------------------------------------- LGTM aside from that below. My head hurts though :). ::: dom/camera/DOMCameraControl.cpp @@ +820,5 @@ > const Optional<OwningNonNull<CameraStartRecordingCallback> >& aOnSuccess, > const Optional<OwningNonNull<CameraErrorCallback> >& aOnError, > ErrorResult& aRv) > { > + DOM_CAMERA_LOGT("%s:%d : this=%p\n", __func__, __LINE__, this); Doesn't OnCreatedFileDescriptor need to check mCameraControl too? And free the descriptor if it doesn't exist?

to test your patch(attachment 8529964 [details] [diff] [review]), i used latest code(master) on today, but there are some compile error. (gecko revision : 59052ffb3b6ef42a1e23f3c0a489e977956263f6)

i thinks there are 3 problems that make memory leak #1. DOMCameraContorls never removes listener. (STR : Comment #0) #2. CameraControls always created (STR : Comment #13) #3. Just doing change visibility (STR : Comment #13), CameraThread is also created every times. (http://dxr.mozilla.org/mozilla-central/source/dom/camera/CameraControlImpl.cpp#34) $ adb shell b2g-ps -t | grep "Camera" APPLICATION SEC USER PID PPID VSIZE RSS WCHAN PC NAME Camera 2 u0_a1968 1968 402 97724 31588 ffffffff b6ee0894 S /system/b2g/b2g CameraThread 2 u0_a1968 2308 1968 97724 31588 c01c378c b6ee0a60 S CameraThread Camera 2 u0_a1968 2367 1968 97724 31588 c01c378c b6ee0a60 S Camera CameraThread 2 u0_a1968 5963 1968 97724 31588 c01c378c b6ee0a60 S CameraThread CameraThread 2 u0_a1968 6260 1968 97724 31588 c01c378c b6ee0a60 S CameraThread CameraThread 2 u0_a1968 6321 1968 97724 31588 c01c378c b6ee0a60 S CameraThread CameraThread 2 u0_a1968 6385 1968 97724 31588 c01c378c b6ee0a60 S CameraThread CameraThread 2 u0_a1968 6447 1968 97724 31588 c01c378c b6ee0a60 S CameraThread CameraThread 2 u0_a1968 6509 1968 97724 31588 c01c378c b6ee0a60 S CameraThread when i reviews your patch(attachment 8529964 [details] [diff] [review]), i think it is only considers #1 and #2. #3 is also has a chance making troubles or memory leaks.

(In reply to jeremy.kim.leo [:jeremykimleo] from comment #18) > i thinks there are 3 problems that make memory leak > > #1. DOMCameraContorls never removes listener. (STR : Comment #0) > #2. CameraControls always created (STR : Comment #13) > #3. Just doing change visibility (STR : Comment #13), CameraThread is also > created every times. ... > #3 is also has a chance making troubles or memory leaks. Thank you for identifying that issue. I've confirmed that old CameraThreads seem to live indefinitely (mRefCnt=4). Investigating.

It turns out that nsThreads are not weak-referenceable, so I've replaced it with a strong reference that persists until the end of the process. I've also verified that this doesn't leak threads. jeremy.kim.leo, please test this and let me know. try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=10b46f7bdfbd

I applied patch v1 and v2 in master branch. But there are still some problems appear. Case 1. Not destroyed GlobalWindow object. - Reproduce : Camera Launch -> Home -> SMS Launch -> New Mesage -> Open Camera Activity(Attach) -> activity close (3 times) - Problem : GlobalWindow and CameraControler still is not destroyed. ================================================================================================ 11-29 13:37:51.640 4179 4179 I CAMERA__: nsGlobalWindow : |b3af8e20| <-- Camera Launch 11-29 13:37:51.960 4179 4179 I CAMERA__: nsGlobalWindow : |b3af9560| 11-29 13:37:52.000 4179 4179 I CAMERA__: nsGlobalWindow : |b3af9ad0| 11-29 13:37:57.570 4179 4179 I CAMERA__: nsGlobalWindow : |b3af9ca0| 11-29 13:37:58.190 4179 4179 I CAMERA__: nsDOMCameraManager : |b2301b50| 11-29 13:37:58.200 4179 4179 I CAMERA__: nsDOMCameraControl : |b6a944c0| 11-29 13:38:05.010 4179 4179 I CAMERA__: ~nsGlobalWindow : |b3af9560| 11-29 13:38:05.010 4179 4179 I CAMERA__: ~nsGlobalWindow : |b3af9ad0| 11-29 13:38:20.780 4179 4179 I CAMERA__: nsGlobalWindow : |b3af9560| <-- Camera Activity Open 11-29 13:38:21.020 4179 4179 I CAMERA__: nsGlobalWindow : |b3af9ad0| 11-29 13:38:21.060 4179 4179 I CAMERA__: nsGlobalWindow : |b3afc9f0| 11-29 13:38:21.640 4179 4179 I CAMERA__: nsDOMCameraManager : |afd74f70| 11-29 13:38:21.650 4179 4179 I CAMERA__: nsDOMCameraControl : |b0680780| 11-29 13:38:27.730 4179 4179 I CAMERA__: ~nsGlobalWindow : |b3af9ad0| 11-29 13:38:30.680 4179 4179 I CAMERA__: ~nsDOMCameraManager : |afd74f70| <-- Activity Close 11-29 13:39:01.350 4179 4179 I CAMERA__: nsGlobalWindow : |afdc8170| <-- Camera Activity Open 11-29 13:39:01.470 4179 4179 I CAMERA__: nsGlobalWindow : |afdc8340| 11-29 13:39:01.520 4179 4179 I CAMERA__: nsGlobalWindow : |afdc8510| 11-29 13:39:02.180 4179 4179 I CAMERA__: nsDOMCameraManager : |aefe5100| 11-29 13:39:02.190 4179 4179 I CAMERA__: nsDOMCameraControl : |b0681080| 11-29 13:39:08.560 4179 4179 I CAMERA__: ~nsGlobalWindow : |afdc8340| 11-29 13:39:15.300 4179 4179 I CAMERA__: ~nsDOMCameraManager : |aefe5100| <-- Activity Close 11-29 13:39:34.300 4179 4179 I CAMERA__: nsGlobalWindow : |afdc8340| <-- Camera Activity Open 11-29 13:39:34.430 4179 4179 I CAMERA__: nsGlobalWindow : |afdc86e0| 11-29 13:39:34.460 4179 4179 I CAMERA__: nsGlobalWindow : |afdc88b0| 11-29 13:39:35.050 4179 4179 I CAMERA__: nsDOMCameraManager : |b342cb50| 11-29 13:39:35.060 4179 4179 I CAMERA__: nsDOMCameraControl : |b06808a0| 11-29 13:39:42.200 4179 4179 I CAMERA__: ~nsGlobalWindow : |afdc86e0| 11-29 13:39:48.630 4179 4179 I CAMERA__: ~nsDOMCameraManager : |b342cb50| <-- Activity Close ================================================================================================ Case 2. Not destroyed CameraControlers - Reproduce : Camera Launch -> Visible change : [Hide(Home) -> Show(Camera)] (5 Times) - Problem : Your patch has a effect to remove the DOMCameraControl object. In this test, Five CameraControlers were created. But only 3 CameraControlers are destroyed. The remained one is referenced by camera app. but another one has to be destroyed. ================================================================================================ 11-29 13:36:14.830 2929 2929 I CAMERA__: nsGlobalWindow : |b3af8e20| <-- Camera Launch 11-29 13:36:15.150 2929 2929 I CAMERA__: nsGlobalWindow : |b3af9560| 11-29 13:36:15.200 2929 2929 I CAMERA__: nsGlobalWindow : |b3af9ad0| 11-29 13:36:22.480 2929 2929 I CAMERA__: nsGlobalWindow : |b3af9ca0| 11-29 13:36:23.160 2929 2929 I CAMERA__: nsDOMCameraManager : |b240cb80| 11-29 13:36:23.170 2929 2929 I CAMERA__: nsDOMCameraControl : |b6a944c0| 11-29 13:36:27.370 2929 2929 I CAMERA__: ~nsGlobalWindow : |b3af9560| 11-29 13:36:27.370 2929 2929 I CAMERA__: ~nsGlobalWindow : |b3af9ad0| 11-29 13:36:52.280 2929 2929 I CAMERA__: nsDOMCameraControl : |b039ee80| <-- Visible : Hide -> Show 11-29 13:36:57.500 2929 2929 I CAMERA__: nsDOMCameraControl : |b039f0c0| <-- Visible : Hide -> Show 11-29 13:37:00.210 2929 2929 I CAMERA__: ~nsDOMCameraControl : |b6a944c0| <-- Visible : Hide 11-29 13:37:02.470 2929 2929 I CAMERA__: nsDOMCameraControl : |b039f1e0| <-- Visible : Show 11-29 13:37:05.640 2929 2929 I CAMERA__: ~nsDOMCameraControl : |b039ee80| <-- Visible : Hide 11-29 13:37:08.130 2929 2929 I CAMERA__: nsDOMCameraControl : |b039ee80| <-- Visible : Show 11-29 13:37:11.870 2929 2929 I CAMERA__: ~nsDOMCameraControl : |b039f0c0| <-- Visible : Hide ================================================================================================ Case 3. CameraThread infinite creation. - Reproduce : Camera Launch -> Visible change : [Hide(Home) -> Show(Camera)] (5 Times) - Problem : I applied patch v2. But CameraThread is still created. ================================================================================================ u0_a10088 10088 507 131572 32812 ffffffff b6f838dc S /system/b2g/b2g u0_a10088 10089 10088 131572 32812 c028a0b8 b6f838dc S Chrome_ChildThr u0_a10088 10090 10088 131572 32812 c01d5098 b6f83aa8 S Analysis Helper u0_a10088 10091 10088 131572 32812 c01d5098 b6f83aa8 S Analysis Helper u0_a10088 10092 10088 131572 32812 c01d5098 b6f83aa8 S Analysis Helper u0_a10088 10093 10088 131572 32812 c01d5098 b6f83aa8 S Analysis Helper u0_a10088 10094 10088 131572 32812 c01d5098 b6f83aa8 S Analysis Helper u0_a10088 10095 10088 131572 32812 c01d5098 b6f83aa8 S Analysis Helper u0_a10088 10096 10088 131572 32812 c01d5098 b6f83aa8 S Analysis Helper u0_a10088 10097 10088 131572 32812 c01d5098 b6f83aa8 S Analysis Helper u0_a10088 10098 10088 131572 32812 c025f4c8 b6f83964 S Socket Thread u0_a10088 10099 10088 131572 32812 c01d5098 b6f83aa8 S BgHangManager u0_a10088 10100 10088 131572 32812 c025f4c8 b6f83964 S MemoryPressure u0_a10088 10101 10088 131572 32812 c01d5098 b6f83aa8 S Timer u0_a10088 10102 10088 131572 32812 c01d5098 b6f83aa8 S ImageBridgeChil u0_a10088 10103 10088 131572 32812 c01d5098 b6f83aa8 S BufferMgrChild u0_a10088 10104 10088 131572 32812 c0666edc b6f8272c S Binder_1 u0_a10088 10105 10088 131572 32812 c0666edc b6f8272c S Binder_2 u0_a10088 10517 10088 131572 32812 c01d5098 b6f83aa8 S CameraThread u0_a10088 10540 10088 131572 32812 c01d5098 b6f83aa8 S HTML5 Parser u0_a10088 10599 10088 131572 32812 c0666edc b6f8272c S Binder_3 u0_a10088 10615 10088 131572 32812 c01d5098 b6f83aa8 S CameraThread u0_a10088 10720 10088 131572 32812 c01d5098 b6f83aa8 S CameraThread u0_a10088 10822 10088 131572 32812 c01d5098 b6f83aa8 S CameraThread u0_a10088 10921 10088 131572 32812 c01d5098 b6f83aa8 S CameraThread ================================================================================================

I made a patch for the CameraThread(Case 3). How about this patch?

Comment on attachment 8530456 [details] [diff] [review] Fix memory leak in CameraControl, v2 please check comment #21, comment #22 remained by my co-worker. there are no effect between before and after with patches.

(In reply to Youngsu Rho from comment #21) > I applied patch v1 and v2 in master branch. > But there are still some problems appear. ... > Case 2. Not destroyed CameraControlers > - Reproduce : Camera Launch -> Visible change : [Hide(Home) -> > Show(Camera)] (5 Times) > - Problem : Your patch has a effect to remove the DOMCameraControl object. > In this test, Five CameraControlers were created. But only 3 > CameraControlers are destroyed. > The remained one is referenced by camera app. but another one > has to be destroyed. This is normal. The cycle-collector is non-deterministic, so it's likely that a few DOMCameraControl objects may exist at the end of your test. I've observed that all but the last instance eventually get destroyed, and the count of outstanding instances doesn't exceed 3. > Case 3. CameraThread infinite creation. > - Reproduce : Camera Launch -> Visible change : [Hide(Home) -> > Show(Camera)] (5 Times) > - Problem : I applied patch v2. But CameraThread is still created. This is actually the test case I've been using to find the leaks, and following these steps, I only ever get a single "CameraThread". I will investigate case 1.

youngsu.rho, my apologies--the v2 patch was missing some additional changes. Please try this patch, which includes all fixes to address cases 1 through 3.

try push: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8334f6b01a43

Fix broken builds: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=2988a0d288f7

Thank you Mike. Works well your patch v3.1. All cases was resolved. Can you make the patch for gecko v2.0 branch?

Fix broken tests. try push: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=edfd8f376477

Comment on attachment 8531102 [details] [diff] [review] Fix memory leak in CameraControl, v3.2 We should nudge the app guys towards dropping callbacks, so we can too. The addition of promises has broken callbacks for methods that can also throw exceptions.

(In reply to Youngsu Rho from comment #28) > Can you make the patch for gecko v2.0 branch? Yes, but it will take a few days.

Wilson, would you mind opening a new bug for your patch and blocking this one on it? I have two versions of patch to prepare for this bug, and I think everything will land cleaner if the steps are separate.

Comment on attachment 8531743 [details] pull-request (master) Moved this patch to its own bug 1107284.

youngsu.rho, please try this version of the patch on v2.0. v2.2/master is quite different, so it's possible the port has introduced new issues.

Updated patch for v2.0, fixes an XPCOM error that was preventing the camera from starting.

Comment on attachment 8532113 [details] [diff] [review] Fix memory leak in CameraControl, v3.3.1 [v2.0/b2g32] LGTM on b2g32 (flame, v2.0) Thanks Mike.

Comment on attachment 8532113 [details] [diff] [review] Fix memory leak in CameraControl, v3.3.1 [v2.0/b2g32] Review of attachment 8532113 [details] [diff] [review]: ----------------------------------------------------------------- r+ with nits addressed. ::: dom/camera/DOMCameraControl.cpp @@ +725,5 @@ > } > > + if (!mCameraControl) { > + rv = NS_ERROR_NOT_AVAILABLE; > + } What happens if mCameraControl == null at line 721? I think this check is in the wrong place.

Dear. Mike i found strange code. look at this int32_t nsDOMCameraControl::SensorAngle() { - MOZ_ASSERT(mCameraControl); - int32_t angle = 0; - mCameraControl->Get(CAMERA_PARAM_SENSORANGLE, angle); + if (!mCameraControl) { + mCameraControl->Get(CAMERA_PARAM_SENSORANGLE, angle); + } return angle; } i think it should be fixed like this + if (mCameraControl) { + mCameraControl->Get(CAMERA_PARAM_SENSORANGLE, angle); + } thanks

Rebased on top of attachment 8534184 [details] [diff] [review].

Minor clean-up.

Fix logic error in DCC::SensorAngle() (thanks, :jeremykimleo!). Carry r+ forward.

try push: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=b7c208853d56

Comment on attachment 8534192 [details] [diff] [review] Fix memory leak in CameraControl, v3.4.1 Review of attachment 8534192 [details] [diff] [review]: ----------------------------------------------------------------- LGTM.

Remove stale class declaration.

Comment on attachment 8534693 [details] [diff] [review] Fix memory leak in CameraControl, v3.4.1 [v2.0/b2g32] r=aosmond NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): memory leak User impact if declined: with certain steps, the Camera app memory use can grow without bounds, leading to performance issues, and the app eventually getting LMKed Testing completed: this bug's STR on Flame; analysis with b2g-ps and get_gc_cc_log.py; normal app usage Risk to taking this patch (and alternatives if risky): there may be regressions to some corner use-cases, but general app operation is correct. String or UUID changes made by this patch: none

Piggybacking a try run: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=c0b245b244d8

[Blocking Requested - why for this release]: memory leak, progressive degradation of device, Camera app eventually LMKed.

This should fix an assertion-count failure in the DEBUG automated tests. (We'll know as soon as the try tree reopens.) Carrying r+ forward.

[Approval Request Comment] see comment 47. v3.5 patch rebased for v2.0/b2g32.

try push: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8b1f2b4f3f74

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c327ed6873ff This needs rebasing for b2g34. Also, take note of a follow-up push Ehsan did on trunk: https://hg.mozilla.org/integration/mozilla-inbound/rev/1c0a42855a50 AFAICT, it wasn't needed for the b2g32 patch, but do include it with the b2g34 patch if applicable there.

Backed out from b2g32 for B2G mochitest crashes. https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/12aea1649f5a https://treeherder.mozilla.org/ui/logviewer.html#?job_id=70793&repo=mozilla-b2g32_v2_0

Fix test failures. [Approval Request Comment] See previous a+. b2g32_v2_1 version coming next.

Fix test bustage; carry r+ forward. [Approval Request Comment] See previous a+. b2g32_v2_0 version coming next (for real).

Fix test bustage; carry r+ forward. [Approval Request Comment] See previous a+.

Comment on attachment 8537023 [details] [diff] [review] Fix memory leak in CameraControl, v3.7 [v2.1/b2g34] r=aosmond Actually, I just noticed that at some point in the backport, there was a bad merge, and both this and the v2.1 patch are missing some critical code. Dropping for now.

Comment on attachment 8537029 [details] [diff] [review] Fix memory leak in CameraControl, v3.7 [v2.0/b2g32] r=aosmond Ditto.

Remove declaration of symbols not required in backport; carrying r+ forward. [Approval Request Comment] See previous a+.

Remove declaration of symbols not required in backport; carrying r+ forward. [Approval Request Comment] See previous a+.

Fix editing mistake; carrying r+ forward. Seems I only notice things in the bugzilla diff viewer. :-/ [Approval Request Comment] See previous a+.

Fix typo; carry r+ forward; GO TO SLEEP... and never again work on parallel backports after midnight again. [Approval Request Comment] See previous a+.

Comment on attachment 8537043 [details] [diff] [review] Fix memory leak in CameraControl, v3.7.1 [v2.1/b2g34] r=aosmond No need to re-request approval unless you've made changes to the patch that substantially change your answers to the approval questionnaire. Given that you're just referring back to those original answers, it doesn't sound like that's the case :)

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/2548deff272b https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/01e5265fde4a

Applies on top of attachment 8537051 [details] [diff] [review]. b2g34 version coming next.

v2.0: https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/777195b0a209 v2.1: https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/7b161e9fe25e

Hitting asserts now :( https://treeherder.mozilla.org/ui/logviewer.html#?job_id=55105&repo=mozilla-b2g34_v2_1 https://treeherder.mozilla.org/ui/logviewer.html#?job_id=72343&repo=mozilla-b2g32_v2_0 Backed out.

Andrew, would you mind reviewing this backport? It's a pretty significant change. The upside is, it works! try push: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=df7e5df53e85

Comment on attachment 8538167 [details] [diff] [review] Fix memory leak in CameraControl, v3.8 [v2.0/b2g32] Review of attachment 8538167 [details] [diff] [review]: ----------------------------------------------------------------- LGTM.

Carrying r+ from related patches. try push: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=df9ca6edb1dc

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/d41f6bd64343 https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/f57067657bfa

This issue has been successfully verified on Flame v2.0&2.1&2.2 and Woodduck 2.0. See attachment: verified_v2.2.MP4 and memoryUsage_After30times_v2.1.txt. Reproduce rate: 0/35. STR: 1. Launch Camera app and then tap Home button to go back to homescreen. 2. Launch Messages app. 3. New message => attachment => camera. 4. Take a picture => select. 5. Back from New messages => discard. 6. Repeat step 3&4&5. 7. Repeat step 3&4&5... Actual results: Memory usage of camera app is stable. After repeated 30 times, the memory usage of Camera is "RSS: 23.8", "PSS: 15.1". Please see attachment "memoryUsage_After30times_v2.1.txt" about the memory usage. Woodduck 2.0 build: Gaia-Rev afa87cffbd3cd9e2070b26d45dd556a9324bd4d5 Gecko-Rev 911e6cd6aecf8d37d42c203e162847b78a68a8d8 Build-ID 20141224050313 Version 32.0 Device-Name jrdhz72_w_ff FW-Release 4.4.2 FW-Incremental 1419368730 FW-Date Wed Dec 24 05:05:52 CST 2014 Flame 2.0 build: Gaia-Rev ce83ea7b8e3fa2d1c3fd771fc22b654c18b3c381 Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/4dd6d9b58f42 Build-ID 20141223000202 Version 32.0 Device-Name flame FW-Release 4.4.2 FW-Incremental eng.cltbld.20141223.034247 FW-Date Tue Dec 23 03:42:58 EST 2014 Bootloader L1TC00011880 Flame 2.1 build: Gaia-Rev 17c7ad2e4919a994f0844239b483116090412dee Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/39dfb662c82a Build-ID 20141223001203 Version 34.0 Device-Name flame FW-Release 4.4.2 FW-Incremental eng.cltbld.20141223.035107 FW-Date Tue Dec 23 03:51:18 EST 2014 Bootloader L1TC00011880 Flame 2.2 build: Gaia-Rev c2da2bafd4e809317e2ca70c9bf5c11136a32818 Gecko-Rev https://hg.mozilla.org/mozilla-central/rev/0532f2509f3f Build-ID 20141223010202 Version 37.0a1 Device-Name flame FW-Release 4.4.2 FW-Incremental eng.cltbld.20141223.043429 FW-Date Tue Dec 23 04:34:39 EST 2014 Bootloader L1TC00011880

This issue is verified not happen on latest build of Woodduck 2.0,Flame 2.0/2.1/2.2 See attachment: Verify_video.3gp Reproducing rate: 0/2 When repeated 30 times, the memory usage is Rss: 29.7K, Pss: 19.9K.(Woodduck 2.0) Rss: 34.6K, Pss: 20.3K.(Flame 2.0) Rss: 21.2K, Pss: 13.9K.(Flame 2.1) Rss: 28.7K, Pss: 20.8K.(Flame 2.2) When repeated first time,the memory usage is Rss: 30.5K, Pss: 18.2K.(Woodduck 2.0) Rss: 33.3K, Pss: 19.0K.(Flame 2.0) Rss: 23.0K, Pss: 15.4K.(Flame 2.1) Rss: 26.0K, Pss: 16.5K.(Flame 2.2) Flame 2.0 Build: Gaia-Rev ce83ea7b8e3fa2d1c3fd771fc22b654c18b3c381 Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/4dd6d9b58f42 Build-ID 20141223000202 Version 32.0 ---------------------- Flame 2.1 Build: Gaia-Rev 17c7ad2e4919a994f0844239b483116090412dee Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/39dfb662c82a Build-ID 20141223001203 Version 34.0 ---------------------- Flame 2.2 Build: Gaia-Rev c2da2bafd4e809317e2ca70c9bf5c11136a32818 Gecko-Rev https://hg.mozilla.org/mozilla-central/rev/0532f2509f3f Build-ID 20141223010202 Version 37.0a1 ---------------------- Woodduck 2.0 Build: Gaia-Rev afa87cffbd3cd9e2070b26d45dd556a9324bd4d5 Gecko-Rev 911e6cd6aecf8d37d42c203e162847b78a68a8d8 Build-ID 20141224050313 Version 32.0