js_CheckRedeclaration does not always unlock object on failures

VERIFIED FIXED

Status

()

Core
JavaScript Engine
VERIFIED FIXED
9 years ago
8 years ago

People

(Reporter: Igor Bukanov, Assigned: Igor Bukanov)

Tracking

(4 keywords)

Trunk
crash, testcase, verified1.9.0.9, verified1.9.1
Points:
---
Bug Flags:
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(1 attachment)

(Assignee)

Description

9 years ago
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
(Assignee)

Comment 1

9 years ago
The fix for the bug 476049 has landed in TM and it also fixes this issue.
Whiteboard: fixed-in-tracemonkey
(Assignee)

Comment 2

9 years ago
Nominating for 1.9.0
Flags: wanted1.9.0.x?
(Assignee)

Comment 3

9 years ago
The fix for the bug 476049 has landed in mozilla-central and it also fixes this issue.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 4

9 years ago
Created attachment 362941 [details]
js1_8/extensions/regress-475971.js

Updated

9 years ago
Flags: in-testsuite+
Flags: in-litmus-
(Assignee)

Comment 5

8 years ago
fixed for 1.9.0 as the fix for bug 476049 is landed on this branch
Keywords: fixed1.9.0.9
(Assignee)

Updated

8 years ago
Flags: wanted1.9.0.x?
Whiteboard: fixed-in-tracemonkey → [sg:critical?] fixed-in-tracemonkey
(Assignee)

Comment 6

8 years ago
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?
Keywords: fixed1.9.0.9 → verified1.9.0.9

Comment 8

8 years ago
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.

Comment 10

8 years ago
k.
Group: core-security

Comment 11

8 years ago
bob: test checked in?

Comment 12

8 years ago
no, not yet.

Comment 13

8 years ago
shouldn't this be "in-testsuite -" then?

Comment 14

8 years ago
No. It has a test attached to the bug and that will be checked in shortly.

Comment 15

8 years ago
hi bob.  did it get checked in?

Comment 16

8 years ago
js1_8/extensions/regress-475971.js	
http://hg.mozilla.org/tracemonkey/rev/b399794774c4
Keywords: fixed1.9.1

Comment 17

8 years ago
/cvsroot/mozilla/js/tests/js1_8/extensions/regress-475971.js,v  <--  regress-475971.js
initial revision: 1.1

Comment 18

8 years ago
v no crashes with 1.9.1, 1.9.2, 1.9.3
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
You need to log in before you can comment on or make changes to this bug.