[PATCH 3/3] jscript: Release all globals when the script ctx is destroyed or re-initialized.

[Gabriel Ivăncescu]

Hi Jacek,

On 30/05/2022 21:18, Jacek Caban wrote:
> On 5/30/22 19:24, Gabriel Ivăncescu wrote:
>> Most of these globals were leaking before as they were never freed at
>> all. Also, they have to be freed during script ctx destruction because an
>> unintialized script might still make use of them (e.g. retrieving a 
>> builtin
>> function via PROPERTYGET requires ctx->function_constr to be available),
>> so freeing them during state transition would crash.
> 
> 
> I checked it (see the attached patch) and in such case function 
> prototype is not really functional on Windows. This means that 
> ctx->function_constr is not really needed for them. I didn't test it 
> further, but I wouldn't be surprised if on Windows, all objects would be 
> "detached" at this point from both ctx and prototype.
>
I can't test it right now, will do more tests tomorrow, but AFAIK 
GetDispID doesn't work on native, that's why I retrieve it before 
changing state (even for "length"), although obtaining it after via the 
same DISPID works.

Can you also try to retrieve "call" before setting it to uninitialized 
and then using the DISPID? If it returns a VT_DISPATCH with PROPERTYGET 
it means it works I guess...

>> +static inline void globals_release(script_ctx_t *ctx)
>> +{
>> +    jsdisp_t **iter = &ctx->function_constr, **end = 
>> &ctx->set_prototype + 1;
>> +    while(iter != end) {
>> +        if(*iter) {
>> +            jsdisp_release(*iter);
>> +            *iter = NULL;
>> +        }
>> +        iter++;
>> +    }
>> +}
> 
> 
> That's ugly. We could potentially store those in array in the first 
> place if we really need something like this. Also, there is no need for 
> inline.
> 
> 
> Thanks,
> 
> Jacek

I made it inline because I wanted to place it in the header right below 
the struct so it's kept in sync. I agree an array would be nicer for 
releasing them, but it would make the rest of the code a lot uglier IMO.

How about a union of an array and a struct with current fields? (with a 
comment saying to keep its length in sync). That would at least keep the 
rest of the code the same, instead of stuff like:

     ctx->globals[CTX_OBJECT_PROTOTYPE];

which IMO is uglier.