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.