[PATCH v2 2/3] jscript: Don't execute any functions if script state does not allow it.

[Gabriel Ivăncescu]

On 01/06/2022 03:07, Jacek Caban wrote:
> On 5/31/22 16:56, Gabriel Ivăncescu wrote:
>> Instead of only interpreted functions. Property retrievals or setters are
>> allowed though, as long as they are not accessors.
>>
>> Signed-off-by: Gabriel Ivăncescu<gabrielopcode at gmail.com>
>> ---
>>
>> Note that native GetDispID seem to fail when retrieving builtin/prototype
>> functions (?) if script is uninitialized, but retrieving the DISPID 
>> before and
>> using it after it's uninitialized still works, though (that's what the 
>> tests
>> do). I don't think it's worth replicating this GetDispID quirk for the 
>> moment.
> 
> 
> I'm not really concerned about GetDispID quirks, but implication of your 
> choice for the next patch. I think that releasing objects sooner is 
> better, so it would be good to have a better argument for moving that. 
> Your reasoning there is based on details of how you decided to not 
> follow native here.
> 
> 
> More generally, creating function objects in response for property get 
> is an implementation detail that supposed to be transparent. If it's 
> causing problems elsewhere, I'd first look at making it more 
> transparent. In this case, maybe we should create function objects 
> sooner, in response to GetDispID. This would both match all tests and 
> allow us to release function constructor sooner.
> 
> 
> Jacek
> 

Right, but how is releasing such objects not an implementation detail as 
well? The thing is, there's a very rare crash in FFXIV Launcher when 
logging in that happens when some proxy prototype tries to obtain the 
object prototype, which for some reason is NULL (suggesting 
uninitialized script). I haven't been able to investigate it in depth 
since I wasn't able to reproduce it since then. But it would be neatly 
solved by this as well without adding any complexity.

What's such a big deal about releasing them later? It's not like scripts 
stay that long in the uninitialized state, nor does it complicate the 
code, and it's an implementation detail anyway. If the function object 
was created earlier, it would still use the same function object, so the 
end result is exactly the same.

On the other hand, obtaining the function object on dispid seems like 
too much work for no real gain. On top of work, that would also lose all 
the current optimizations with builtins (which are only retrieved now on 
PROPERTYGET, which is pretty rare—most of the time they're just called 
directly, while looking up their DISPIDs happens all the time). I'm 
definitely not certain that's a good idea.