620178 - _newJSDContext calls JS_EndRequest with null jsdc->dumbContext on certain failures and leaks call on others

Status: RESOLVED DUPLICATE of bug 880697

(Core :: JavaScript Engine, defect)

Description

[timeless]

87 _newJSDContext(JSRuntime*         jsrt, 

92     JSDContext* jsdc = NULL;
93     JSCrossCompartmentCall *call = NULL;

101     jsdc = (JSDContext*) calloc(1, sizeof(JSDContext));
102     if( ! jsdc )
103         goto label_newJSDContext_failure;

105     if( ! JSD_INIT_LOCKS(jsdc) )
106         goto label_newJSDContext_failure;

108     JS_INIT_CLIST(&jsdc->links);

110     jsdc->jsrt = jsrt;

121     JS_INIT_CLIST(&jsdc->threadsStates);
122     JS_INIT_CLIST(&jsdc->sources);
123     JS_INIT_CLIST(&jsdc->removedSources);

127     if( ! jsd_CreateAtomTable(jsdc) )
128         goto label_newJSDContext_failure;
129 
130     if( ! jsd_InitObjectManager(jsdc) )
131         goto label_newJSDContext_failure;
132 
133     if( ! jsd_InitScriptManager(jsdc) )
134         goto label_newJSDContext_failure;

136     jsdc->dumbContext = JS_NewContext(jsdc->jsrt, 256);
137     if( ! jsdc->dumbContext )
138         goto label_newJSDContext_failure;

this is skipped:
140     JS_BeginRequest(jsdc->dumbContext);
141 
142     jsdc->glob = JS_NewCompartmentAndGlobalObject(jsdc->dumbContext, &global_class, NULL);
143 
144     if( ! jsdc->glob )
145         goto label_newJSDContext_failure;
146 
147     call = JS_EnterCrossCompartmentCall(jsdc->dumbContext, jsdc->glob);
148     if( ! call )
149         goto label_newJSDContext_failure;

I have absolutely no idea about this, it's probably leaked when we fail here:

151     if( ! JS_InitStandardClasses(jsdc->dumbContext, jsdc->glob) )
152         goto label_newJSDContext_failure;
153 
154     if( call )
155         JS_LeaveCrossCompartmentCall(call);

this is a safe time to call JS_EndRequest:
157     JS_EndRequest(jsdc->dumbContext);

168 label_newJSDContext_failure:
169     if( jsdc ) {
170         jsd_DestroyObjectManager(jsdc);
171         jsd_DestroyAtomTable(jsdc);

this is not safe in a bunch of cases:
172         JS_EndRequest(jsdc->dumbContext);

Comment 1

[timeless]

Attachment: cleanup cleanup

Comment 2

[timeless]

Attachment: updated

Comment 3

[Jason Orendorff [:jorendorff]]

Comment on attachment 523274 [details] [diff] [review]
updated

timeless, are you still working on mozilla stuff? If you're not but you would like me to push this, I can.

Comment 4

[Dan Gohman [:sunfish]]

dumbContext was removed in bug 880697. Can this bug be closed now?

Comment 5

[Andrei Purice]

Hello,
Can you still reproduce this issue or should we close it?

Comment 6

[timeless]

Andrei: I can't politely explain why the approach you're taking is bad.

Dan was right, removing this buggy code is a way to fix it.

As it's a strict code analysis bug, it shouldn't have been necessary to ask me for information.