935814 - CID 749215, CID 1123244, CID 1123245: Resource leaks on OOM, in jsd as found by Coverity

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Coverity analysis of Oct 31, 2013 source code has found some "resource leaks" that might be the cause of some OOM problems. I'm not sure if every one of them is a problem, though.

http://hg.mozilla.org/mozilla-central/annotate/9ba3faa35c96/js/jsd/jsd_xpc.cpp#l2861 - rec is not being freed here in jsdService::AppendFilter?

http://hg.mozilla.org/mozilla-central/annotate/9ba3faa35c96/js/jsd/jsd_xpc.cpp#l1262 - ret is not being freed here in jsdScript::GetParameterNames? (blame probably points to bz)

http://hg.mozilla.org/mozilla-central/annotate/9ba3faa35c96/js/jsd/jsd_xpc.cpp#l732 - ds is not being freed here in jsds_ScriptHookProc?

jimb, are all 3 of them likely issues?

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

These are CIDs 749215, 1123244 and 1123245.

Comment 2

[Jim Blandy :jimb]

(In reply to Gary Kwong [:gkw] [:nth10sd] (yes, still catching up on bugmail) from comment #0)
> Coverity analysis of Oct 31, 2013 source code has found some "resource
> leaks" that might be the cause of some OOM problems. I'm not sure if every
> one of them is a problem, though.
> 
> http://hg.mozilla.org/mozilla-central/annotate/9ba3faa35c96/js/jsd/jsd_xpc.
> cpp#l2861 - rec is not being freed here in jsdService::AppendFilter?

There, it looks to me as if rec is always either freed, or added to the gFilters linked list.


> http://hg.mozilla.org/mozilla-central/annotate/9ba3faa35c96/js/jsd/jsd_xpc.
> cpp#l1262 - ret is not being freed here in jsdScript::GetParameterNames?
> (blame probably points to bz)

This one looks okay too. There are only two ways to exit after allocating ret: line 1259 (failure), and line 1262 (success). On success, we return the array in *paramNames, so that's not a leak. The only way to return via 1259 is to set rv to a failure result - and that can only happen if we just called NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY, which seems to free ret.


> http://hg.mozilla.org/mozilla-central/annotate/9ba3faa35c96/js/jsd/jsd_xpc.
> cpp#l732 - ds is not being freed here in jsds_ScriptHookProc?

This is another case like the first: ds seems to be always saved in gDeadScripts. Perhaps Coverity couldn't figure out our doubly-linked list implementation.


> jimb, are all 3 of them likely issues?

It looks to me as if none of them are leaks.

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

> It looks to me as if none of them are leaks.

Understood - I've already marked them as false positives. Thanks!