Closed Bug 1261983 Opened 7 years ago Closed 6 years ago

[infer] Resource leaks in dlc

Categories

(Firefox for Android Graveyard :: General, defect)

Unspecified
Android
defect
Not set
normal

Tracking

(firefox55 fixed)

RESOLVED FIXED
Firefox 55
Tracking Status
firefox55 --- fixed

People

(Reporter: mcomella, Assigned: ahunt, Mentored)

References

Details

Attachments

(1 file)

/var/lib/jenkins/workspace/fennec-infer/mobile/android/base/java/org/mozilla/gecko/dlc/BaseAction.java:142: error: RESOURCE_LEAK
   resource acquired by call to FileInputStream(...) at line 115 is not released after line 142

/var/lib/jenkins/workspace/fennec-infer/mobile/android/base/java/org/mozilla/gecko/dlc/DownloadAction.java:190: error: RESOURCE_LEAK
   resource acquired by call to openFile(...) at line 179 is not released after line 190

/var/lib/jenkins/workspace/fennec-infer/mobile/android/base/java/org/mozilla/gecko/dlc/DownloadAction.java:216: error: RESOURCE_LEAK
   resource acquired by call to FileInputStream(...) at line 216 is not released after line 216

/var/lib/jenkins/workspace/fennec-infer/mobile/android/base/java/org/mozilla/gecko/dlc/DownloadAction.java:234: error: RESOURCE_LEAK
   resource acquired by call to FileInputStream(...) at line 216 is not released after line 234

/var/lib/jenkins/workspace/fennec-infer/mobile/android/base/java/org/mozilla/gecko/dlc/DownloadAction.java:306: error: RESOURCE_LEAK
   resource acquired by call to FileInputStream(...) at line 294 is not released after line 306

/var/lib/jenkins/workspace/fennec-infer/mobile/android/base/java/org/mozilla/gecko/dlc/DownloadAction.java:307: error: RESOURCE_LEAK
   resource acquired by call to FileOutputStream(...) at line 295 is not released after line 307

Sebastian, mind taking a look?
Flags: needinfo?(s.kaspari)
Thank you! I'll have a look.
Flags: needinfo?(s.kaspari)
This looks like false positives.

The tool doesn't seem to recognize our helper method IOUtils.safeStreamClose() as a way to close the stream. Doesn't it follow the code inside the methods? Can we teach/configure the tool to do so?
Flags: needinfo?(michael.l.comella)
Example:

> /var/lib/jenkins/workspace/fennec-infer/mobile/android/base/java/org/mozilla/gecko/dlc/DownloadAction.java:190: error: RESOURCE_LEAK
>   resource acquired by call to openFile(...) at line 179 is not released after line 190

Code:
  https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/dlc/DownloadAction.java#189-190
(In reply to Sebastian Kaspari (:sebastian) from comment #2)
> This looks like false positives.
>
> The tool doesn't seem to recognize our helper method
> IOUtils.safeStreamClose() as a way to close the stream. Doesn't it follow
> the code inside the methods? Can we teach/configure the tool to do so?

So they're open source: https://github.com/facebook/infer/ (I hope you've kept up with your OCaml ;)

And they have a bug open for this functionality: https://github.com/facebook/infer/issues/246

That being said:
 * Beyond whitelisting/blacklisting files [1], I can't find a way to ignore individual checks. There is an undocumented `.inferconfig` file – perhaps there's something in there? Worst case, we can filter the output to ignore the checks we don't want to see anymore (we'll have more control over this once we add infer locally but that might be some ways off).
 * The docs mentions dynamic dispatch is something they can't do (yet?) [2] – I wonder if that applies to static methods?

I'm not sure what the best course of action is – maybe just close this bug, filter out these checks in my script, and hope it gets fixed soon? :P

[1]: https://github.com/facebook/infer/issues/312
[2]: http://fbinfer.com/docs/limitations.html
Flags: needinfo?(michael.l.comella)
(In reply to Michael Comella (:mcomella) from comment #4)
> And they have a bug open for this functionality:
> https://github.com/facebook/infer/issues/246

Oops, this is actually specific to code in that issue.

That being said, I found out you can use @SuppressWarnings("infer") [1] at the method level and they should work in gradle on more recent versions – unclear what we're running in "automation".

[1]: https://github.com/facebook/infer/issues/150#issuecomment-141204309
[2]: https://github.com/facebook/infer/issues/150#issuecomment-199320293
Assignee: s.kaspari → nobody
FWIW it looks like most of the issues have disappeared with the current version of infer, however there is still one issue.
Assignee: nobody → ahunt
Attachment #8858401 - Flags: review?(cnevinchen)
Attachment #8858401 - Flags: review?(cnevinchen) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/d5b67af192ba8b8d0abbdb0f03bde9809da626d2
Bug 1261983 - infer: close file input stream in case of GZIP failure r=nechen
https://hg.mozilla.org/mozilla-central/rev/d5b67af192ba
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.