[PATCH resend 03/13] msscript.ocx: Add initial IScriptModule stub implementation.

[Gabriel Ivăncescu]

Hi Jacek,

On 29/05/2020 21:46, Jacek Caban wrote:
> On 29.05.2020 18:52, Jacek Caban wrote:
>> Hi Gabriel,
>>
>> On 26.05.2020 14:41, Gabriel Ivăncescu wrote:
>>> Signed-off-by: Gabriel Ivăncescu<gabrielopcode at gmail.com>
>>> ---
>>>
>>> We have to treat a ref count of 0 in a special manner when they are 
>>> linked
>>> to the script control, because the modules must exist even when they 
>>> have
>>> no external refs (as the script control can still use them).
>>>
>>> However, as evidenced by the tests, they must*not*  keep a ref to the 
>>> script
>>> control in this case, as it would create a circular ref and make no 
>>> sense,
>>> anyway.
>>>
>>> The alternative solution to allocate separate data for modules would
>>> require copying the data to multiple places (one for Module Collection,
>>> and one for orphaned Modules) which is more complicated, in my opinion.
>>
>>
>> There must be a cleaner way. If you need too much special treatment 
>> for orphaned modules, it means that the design needs to be improved. 
>> One thing that is often useful for similar cases are weak reference: 
>> never reference control from module and make sure that control always 
>> invalidates the module->control pointer when it actually releases the 
>> object.

Ok so, I'll give some examples to illustrate why it has to be done 
either this way (with refcount checks), or by allocating separate 
objects for modules when requesting an external ref (on top of the 
objects stored in the collection).

I will send a patch series with the latter (separate objects) to show 
the alternative solution, but it does complicate the code a bit and the 
enumerator as I mentioned initially... that's what I meant the first 
time and why I went with the refcount checks.

So, the tests show that the behavior is like this:

Module Collection is tied to Control. Obtaining a ref to the collection, 
then updating the control via put_Language, and using the older 
collection ref will refer to the new control (i.e. if it had 5 modules, 
then put_Language resets it back to 1, the old collection will also 
report 1 module and refer to it). So keeping it on the control is the 
easiest as it avoids pointless allocations in the code.

Modules are, obviously, tied to the collection (in an array for random 
access by index). A collection needs module names, for example. The 
control also needs access to the first (global) module to simplify the 
code. So modules always exist after put_Language, even if there's no 
explicit ref to them. In this case, they can't ref anything since the 
caller might only hold a ref to the control.

However, consider what happens if the caller/user obtains a ref to a 
module. First of all, this keeps the control alive, so releasing the ref 
to the script control in this case will not destroy it, you can still 
use the module ref to do stuff with it. So, a module obtained externally 
needs to hold a ref to the control.

Secondly, put_Language detaches the modules from the control (or 
collection). They lose their ref to the control, and operations on them 
return E_FAIL.


What I did with this patch series is to hold a refcount of 0 when no 
external (caller/user) refs are held to the module, but only the 
control/collection can access them (and their names). Increasing 
refcount to above 0 will obtain a ref to the control, since it means 
they have an external ref.

put_Language would detach them from the module, releasing the control 
ref only if the ref was above 0 (i.e. they had external ref), otherwise 
it would simply destroy the module, as it means they had no external ref.


The alternative I will send later works like this: keep an array of 
modules (without the interface) that keeps the module data in the 
control/collection. When an external ref is requested, we allocate (if 
it's not already, otherwise we share it) a module interface and have it 
keep a ref to the control and point to the module data in the array. 
put_Language would detach all the allocated objects from the control.

The end result is the same, but personally, I believe it introduces more 
complexity to the code, as we have more allocations to deal with, and 
more error handling (even in the enumerator). :-/

That's why I preferred the refcount check implementation, but anyway, 
I'll do the alternative so you can see how it looks like and if you 
prefer it.

> 
> 
> Or maybe script module should just store ScriptHost reference.
> 

It wouldn't change much and we need a ref to the control anyway (to 
start the script for example). Note that modules that haven't been 
detached by put_Language must keep the control alive if they have 
external refs.

Thanks,
Gabriel