[PATCH v4 04/10] vbscript: Destroy the entire script context when the script engine is closed.

[Jacek Caban]

On 11/7/19 1:12 PM, Gabriel Ivăncescu wrote:
> On 11/6/19 6:47 PM, Jacek Caban wrote:
>> Hi Gabriel,
>>
>>
>> On 11/6/19 12:53 PM, Gabriel Ivăncescu wrote:
>>> When the engine is closed, even the persistent code is removed.
>>>
>>> Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
>>> ---
>>>
>>> Removing the context altogether is necessary when the script context 
>>> will
>>> be ref counted, since we can't re-use the existing one. It is also 
>>> needed
>>> for script persistence to work properly.
>>>
>>> This is required because re-initializng a new context when the old 
>>> one is
>>> closed can potentially fail, and decrease_state shouldn't be able to 
>>> fail.
>>>
>>> So instead it is re-initialized on demand, when necessary.
>>
>>
>> I think I already mentioned it, but it doesn't look right to me. When 
>> releasing the context you lose safety flags that are stored in the 
>> context. I'd be very surprised if that's what should happen. You're 
>> also missing handling of AddNamedItem.
>>
>> Overall, I fail to see why why we need it. It seems to me that if you 
>> go from closed to initialized state, you shouldn't use decrease_state 
>> at all.
>>
>> Thanks,
>> Jacek
>
> Hi Jacek,
>
> It's needed for ref counting the context since we'll need the old 
> persistent code_list referenced while the context can be new. And yeah 
> it was mostly for the TypeInfo, but I have an alternative below.
>
> That said, at the very least, this patch should release the code list 
> (persistent code) without releasing the context in that case, 
> otherwise it doesn't pass the tests at the end of the series.


Yes, I think this should work.


Thanks,

Jacek