Re: msi [1/6]: Add support for remote handles

[Alexandre Julliard]

"James Hawkins" <truiken(a)gmail.com> writes:
> @@ -215,18 +282,28 @@ UINT WINAPI MsiCloseHandle(MSIHANDLE han
>
> EnterCriticalSection( &MSI_handle_cs );
>
> -    info = msihandle2msiinfo(handle, 0);
> -    if( !info )
> -        goto out;
> -
> -    if( info->magic != MSIHANDLE_MAGIC )
> +    unk = msi_get_remote( handle );
> +    if (unk)
> +        IUnknown_Release( unk );
> +    else
>      {

It seems to me you are leaking the object here, the handle entry holds
a ref to it too.

-- 
Alexandre Julliard
julliard(a)winehq.org

msi [1/6]: Add support for remote handles

[James Hawkins]

On 6/21/07, Alexandre Julliard <julliard(a)winehq.org> wrote:
> "James Hawkins" <truiken(a)gmail.com> writes:
> > @@ -215,18 +282,28 @@ UINT WINAPI MsiCloseHandle(MSIHANDLE han
> >
> > EnterCriticalSection( &MSI_handle_cs );
> >
> > -    info = msihandle2msiinfo(handle, 0);
> > -    if( !info )
> > -        goto out;
> > -
> > -    if( info->magic != MSIHANDLE_MAGIC )
> > +    unk = msi_get_remote( handle );
> > +    if (unk)
> > +        IUnknown_Release( unk );
> > +    else
> >      {
>
> It seems to me you are leaking the object here, the handle entry holds
> a ref to it too.

The handle entry only holds a ref to the object if it's a regular
handle, and not remote.  I'm resending tomorrow, because there was a
missing check in alloc_handle_table_entry.

-- 
James Hawkins

msi [1/6]: Add support for remote handles

[Alexandre Julliard]

"James Hawkins" <truiken(a)gmail.com> writes:
> On 6/21/07, Alexandre Julliard <julliard(a)winehq.org> wrote:
> > It seems to me you are leaking the object here, the handle entry holds
> > a ref to it too.
>
> The handle entry only holds a ref to the object if it's a regular
> handle, and not remote.  I'm resending tomorrow, because there was a
> missing check in alloc_handle_table_entry.

I don't mean an MSI ref, but since you hold a pointer to the IUnknown
you have to hold a ref to it, and that's the one you are not
releasing. You'd need two IUnknown_Release calls in there (and it
would probably be nicer to make MsiCloseHandle lookup objects directly
instead of calling helpers that will validate the handle multiple
times).

-- 
Alexandre Julliard
julliard(a)winehq.org