Re: Re: [OLE] Better handling of References

> On Wed, 26 May 2004 03:55:25 +0200, Raphael wrote:
> > I'm trying to install Splinter Cell Demo (installshield).
> >  And i have seen:
> > - one typelib was released while typeinfos (childs of typelib) are in use
> > - the install "crash" (catched and returns an E_UNEXPECTED error) when it
> >
> > wants to call ITypeInfo_fnGetContainingTypeLib (who want to AddRef the 
> > freshly released TypeLib)
>
> Hi Raphael,

Hi Mike
> This has been a known problem for a long time now, with several attempts
> to fix it, all of them have been incorrect. It seems your approach is a
> new one and interesting, but I'm not sure this is how Windows does it.

Well Soft References are used in many places in COM (for example D3D textures/surfaces use it on windows)
> Does anybody know exactly what Windows does in this situation? It might be
> worth writing a test case to find out.
>
> Objects in a semi-freed state seem like a slightly odd way to do things.
> > Now i have another crash later, but it seems a problem with an invoke method 
> > who failed :(
> > If any ole/rpc expert can look at this problem i'll be very happy :)
>
> Well, if you could provide more detail (backtrace etc) on the crash it
> would be good.

I can't have a backtrace as i can't get winedbg working correctly on installshield programs :(
But i'll send you the traces later (when i'll be back home).

For what i have see it's a problem of ITypeInfo::Invoke 
when it should returns a Pointer to an IUnknown Variant.
InstallShield complains about Objects References not set !!
> A few comments on the patch:
> - --(This->ref);
> - /** escape decrement for soft_references */
> - if (This->ref > 0) {
> -
>  --(This->ref);
> - } else {
> -
>  ERR("trying to Release again an ITypeLib2 in freing state waiting for ITypeInfo (%p) childs Releases\n", This);
> - }
>
> I don't remember whether this object is supposed to be thread safe or not,
> but should this not be an InterlockedDecrement?

Yes, but as the code of ITypeLib2/ITypeInfo don't seems thread safe at all ... (anyway i'll fix it) :)
> I think the error could be better phrased as:
>
> ERR("Reference count on an ITypeLib2 object (%p) already zero: waiting for
> ITypeInfo children to be released as well");

:)
> or words to that effect.
> -
>  if (This->soft_ref > 0) {
> - ERR("trying to Release an ITypeLib2 while some ITypeInfo (%p) childs are actually with Refs > 1. So Dont free it now \n", This);
> - /** maybe scaning ITypeInfo list to dump ITypeInfos with Refs  > 1 */
> - return 1;
> -
>  }
>
> Is it actually an error to do that? If you free a typelib while refs are
> held on child typeinfos, is this a bug in the app or a legitimate thing to
> do? If it's not a bug in the app we probably shouldn't use ERR here.

Weel, we had problems like that on d3d8 (a long time ago) about ref counting. 
For me, seems more a wine bug (we have to found where missing AddRefs as to be done) than an app bug (new traces can help us to find what happened)
> --- variant.c	30 Apr 2004 18:32:58 -0000	1.97
> +++ variant.c	26 May 2004 01:46:44 -0000
> @@ -605,6 +605,8 @@
>
> hres = VARIANT_ValidateType(V_VT(pVarg));
> - if (V_VT(pVarg) == VT_EMPTY) return hres;
> - if (SUCCEEDED(hres))
> {
>   if (!V_ISBYREF(pVarg))
>
> This change looks OK though I'm not sure why it's necessary. Does the
> Splinter Cell installer actually need this?

No, i have only to no have a lot of debug traces (i have added on my tree) on Variant* Operations when clearing already empty Variants
> thanks -mike

Regards,
Raphael

[OLE] Better handling of References

[Mike Hearn]

On Wed, 26 May 2004 12:58:19 +0000, fenix wrote:
> Well Soft References are used in many places in COM (for example D3D textures/surfaces use it on windows)

OK :) I have not seen this technique before, but then I've never
implemented lots of COM objects all working together.
> I can't have a backtrace as i can't get winedbg working correctly on
> installshield programs :( But i'll send you the traces later (when i'll
> be back home).

What does winedbg do/not do? I've been able to use it with InstallShield
before: you may have to attach to it after the program has started though.
> For what i have see it's a problem of ITypeInfo::Invoke when it should
> returns a Pointer to an IUnknown Variant. InstallShield complains about
> Objects References not set !!

Yes, this problem has plagued us for ages. It seems to be a race condition
of some sort, Marcus has debugged this extensively.

By "should return a pointer to an IUnknown variant" do you mean that
pVarResult should hold a VT_UNKNOWN variant? Do you know what
function/property it's trying to use?
> Yes, but as the code of ITypeLib2/ITypeInfo don't seems thread safe at
> all ... (anyway i'll fix it) :)

Yeah I know :) It just seems that we might as well write new code to be
thread safe so when somebody comes to make it thread safe they have less
work to do ...
> No, i have only to no have a lot of debug traces (i have added on my
> tree) on Variant* Operations when clearing already empty Variants

OK, that makes sense.

thanks -mike