resolve (most) mac static analyzer issues

[btjones]

This resolves most of the potential (and actual) memory leaks and other issues identified by the Xcode static analyzer. The only analyzer issues that I did not resolve are:

• brackets-shell/appshell/client_handler_mac.mm:419:7: Incorrect decrement of the reference count of an object that is not owned at this point by the caller
• brackets-shell/appshell/client_handler_mac.mm:430:1: Potential leak of an object

The reason I didn't make any changes here are because it appears brackets-shell is overriding a cef window delegate, as explained in the comment at client_handler_mac.mm:411, and the code is decrementing the cef delegate which we don't have ownership of BUT it is intentional. I left that code as-is for now because I did not see a simple way to resolve those two analyzer issues.

After running Instruments and looking for leaks, this update fixes quite a few leaks which were appearing when opening the app.

All Brackets Jasmine tests pass.

[btjones]

There are definitely some other leaks in the mac version of brackets-shell and I'm planning to look into fixing some of those but wanted to send this pull request since this deals solely with those issues identified by the static analyzer.

[ingorichter]

@btjones What other leaks did the static analysis find?

[btjones]

@ingorichter

Brackets target issues:

brackets-shell/appshell/CustomTitlebarView.m:70:5: Potential leak of an object stored into 'lm'
brackets-shell/appshell/CustomTitlebarView.m:96:5: Potential leak of an object
brackets-shell/appshell/FullScreenButton.mm:94:1: Potential leak of an object stored into 'focusTrackingArea'
brackets-shell/appshell/client_handler_mac.mm:122:3: Instance variable used while 'self' is not set to the result of '[(super or self) init...]'
brackets-shell/appshell/client_handler_mac.mm:265:10: Potential leak of an object stored into 'tlController'
brackets-shell/appshell/client_handler_mac.mm:282:6: Potential leak of an object stored into 'fsController'
brackets-shell/appshell/client_handler_mac.mm:419:7: Incorrect decrement of the reference count of an object that is not owned at this point by the caller
brackets-shell/appshell/cefclient_mac.mm:157:5: Instance variable used while 'self' is not set to the result of '[(super or self) init...]'
brackets-shell/appshell/cefclient_mac.mm:326:10: Potential leak of an object stored into 'tvController'
brackets-shell/appshell/cefclient_mac.mm:346:1: Potential leak of an object stored into 'fsController'
brackets-shell/appshell/cefclient_mac.mm:498:3: Returning 'self' while it is not set to the result of '[(super or self) init...]'
brackets-shell/appshell/appshell_extensions_mac.mm:476:12: Potential leak of an object stored into 'testPath'
brackets-shell/appshell/appshell_extensions_mac.mm:951:16: Potential leak of an object stored into 'itemTitleStr'
brackets-shell/appshell/appshell_extensions_mac.mm:1059:16: Potential leak of an object stored into 'itemTitleStr'
brackets-shell/appshell/appshell_extensions_mac.mm:1095:31: Potential leak of an object stored into 'keyStr'
brackets-shell/appshell/appshell_extensions_mac.mm:1144:16: Potential leak of an object stored into 'itemTitleStr'
brackets-shell/appshell/appshell_extensions_mac.mm:1198:5: Potential leak of an object stored into 'keyStr'

appshell_helper_app target issues:
(all of these are duplicates from the Brackets target)

brackets-shell/appshell/CustomTitlebarView.m:70:5: Potential leak of an object stored into 'lm'
brackets-shell/appshell/CustomTitlebarView.m:96:5: Potential leak of an object
brackets-shell/appshell/FullScreenButton.mm:94:1: Potential leak of an object stored into 'focusTrackingArea'
brackets-shell/appshell/client_handler_mac.mm:122:3: Instance variable used while 'self' is not set to the result of '[(super or self) init...]'
brackets-shell/appshell/client_handler_mac.mm:265:10: Potential leak of an object stored into 'tlController'
brackets-shell/appshell/client_handler_mac.mm:282:6: Potential leak of an object stored into 'fsController'
brackets-shell/appshell/client_handler_mac.mm:419:7: Incorrect decrement of the reference count of an object that is not owned at this point by the caller
brackets-shell/appshell/appshell_extensions_mac.mm:476:12: Potential leak of an object stored into 'testPath'
brackets-shell/appshell/appshell_extensions_mac.mm:951:16: Potential leak of an object stored into 'itemTitleStr'
brackets-shell/appshell/appshell_extensions_mac.mm:1059:16: Potential leak of an object stored into 'itemTitleStr'
brackets-shell/appshell/appshell_extensions_mac.mm:1095:31: Potential leak of an object stored into 'keyStr'
brackets-shell/appshell/appshell_extensions_mac.mm:1144:16: Potential leak of an object stored into 'itemTitleStr'
brackets-shell/appshell/appshell_extensions_mac.mm:1198:5: Potential leak of an object stored into 'keyStr'

[btjones]

Added another commit that explicitly releases an object so that it will get released right away instead of waiting on the autorelease pool. Reran all the Brackets Jasmine tests successfully.

[btjones]

Here are before and after screen shots of the leaks in the Instruments app. In this test I just launched Brackets and did not perform any actions. I wanted to keep it simple so we could compare apples to apples.

Before my changes:

After my changes:

[njx]

Hey - sorry for the lack of response. I'll try to raise the visibility of this PR with the team, although we're a bit slammed right now so it might still be a bit before we get to it. Thanks for looking into this.

[ingorichter]

@btjones I'm going to have a look at it right now

[njx]

Might want @JeffryBooher to weigh in as well since I think he wrote most of the code that's modified by this PR.

[JeffryBooher]

This needs extensive testing. The changes look OK from my quick initial pass but I can give this branch a whirl this afternoon

[ingorichter]

I'm going to work with the shell as much as I can to figure out any issues (if any).

[JeffryBooher]

cc: @bobeast

[bobeast]

The changes in the areas I'm familiar with look reasonable.

[ingorichter]

@JeffryBooher @btjones @njx I used the shell now for more than a day without any issues. To me this looks and feels good so far.

[ingorichter]

@JeffryBooher @btjones @njx I feel good about these changes. I've used this shell now for more than a week and I haven't noticed any regression. I'm fine with merging the PR. Please speak up now or I'm going to merge it around noon PST.

[peterflynn]

@JeffryBooher @bchintx Is there a similar static analyzer we could run on Windows? There were enough little leaks found here that it seems highly likely some similar problems exist on Windows too...

[btjones]

On the Mac, Xcode uses Clang Static Analyzer: http://clang-analyzer.llvm.org/

To use it on Windows you will need to build it from source (http://clang-analyzer.llvm.org/installation#OtherPlatforms). I'm guessing there may be something more standard on the Windows side though.

[JeffryBooher]

You can also use the static analysis built in to Visual Studio. There is also Coverity which Adobe has a license for.

[JeffryBooher]

@ingorichter I tested this around the areas I was concerned with and it looks good. I think it's ready to merge.

[ingorichter]

@JeffryBooher @peterflynn Do we want to create a story for Windows to track efforts on static code analysis?

[ingorichter]

Thank you @btjones.

[peterflynn]

@ingorichter I think we should, yeah. Or a general 'memory leak audit' card at least (whether it's static analysis or valgrind or what).

[JeffryBooher]

We could make static analysis part of the daily build.

[ingorichter]

Yes, we could have a separate job to run the analysis more frequently. What tools would we use for Windows?