Closed Bug 475971 Opened 11 years ago Closed 11 years ago

js_CheckRedeclaration does not always unlock object on failures

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

VERIFIED FIXED

People

(Reporter: igor, Assigned: igor)

References

Details

(4 keywords, Whiteboard: [sg:critical?] fixed-in-tracemonkey - fixed by 476049)

Attachments

(1 file)

The contract for js_CheckRedeclaration is to always call OBJ_DROP_PROPERTY on failures. But this does not happen when the caller passes non-null objp/propp. For objects shared between threads this leads to unbalanced js_(Lock/Unlock)Object. The example below demonstrates this. Currently it hangs when run in a thread-safe shell.

Note this probably is not reproducible in the browser as even with thread workers no objects should be shared between threads.  

function x() { return 1; };

// g must run sufficiently long to ensure that the global scope is accessed                                                                                        
// from the parallel threads.                                                                                                                                      
function g()
{
    var sum = 0;
    try {
        for (var i = 0; i != 10000; ++i) {
            sum += x();
        }
    } catch (e) { }
}

scatter([g, g]);

try {
    eval("const x = 1");
} catch (e) { }

scatter([g, g]);

print("Done");
Depends on: 476049
The fix for the bug 476049 has landed in TM and it also fixes this issue.
Whiteboard: fixed-in-tracemonkey
Nominating for 1.9.0
Flags: wanted1.9.0.x?
The fix for the bug 476049 has landed in mozilla-central and it also fixes this issue.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Flags: in-testsuite+
Flags: in-litmus-
fixed for 1.9.0 as the fix for bug 476049 is landed on this branch
Keywords: fixed1.9.0.9
Flags: wanted1.9.0.x?
Whiteboard: fixed-in-tracemonkey → [sg:critical?] fixed-in-tracemonkey
Regarding sg:critical - this bug itself is not critical. The reason that the patch for bug 476049 included the fix was that it was easy to make the patch there that would fix this bug rather than not.
Whiteboard: [sg:critical?] fixed-in-tracemonkey → [sg:critical?] fixed-in-tracemonkey - fixed by 476049
Verified for 1.9.0.9 as bug 476049 has been verified for 1.9.0.9. 

Bob, I don't suppose that this test could be added to the 1.9.0 test runs?
Al, it has been although it is not checked in since it is sensitive. No need to verify js bugs that have in-testsuite+ marked since I'll do them.
All right. I'm just trying to make sure 1.9.0.9 bugs are verified by today since we make builds on Monday and I'd hate to find out that we didn't fix something AFTER we have official builds.
k.
Group: core-security
bob: test checked in?
no, not yet.
shouldn't this be "in-testsuite -" then?
No. It has a test attached to the bug and that will be checked in shortly.
hi bob.  did it get checked in?
js1_8/extensions/regress-475971.js	
http://hg.mozilla.org/tracemonkey/rev/b399794774c4
Keywords: fixed1.9.1
/cvsroot/mozilla/js/tests/js1_8/extensions/regress-475971.js,v  <--  regress-475971.js
initial revision: 1.1
v no crashes with 1.9.1, 1.9.2, 1.9.3
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.