Re: Implement StdMarshal::ReleaseMarshalData

[Jeroen Janssen]

Mike Hearn wrote:
> Mike Hearn 
> Rob Shearman 
> Implement StdMarshal::ReleaseMarshalData
> - res = IRpcStubBuffer_Release(stub);

I'm not sure but, is there a hypothical race condition here when a
thread switch takes place and someone tries to reuse the stub while
valid is not set to TRUE yet (but the stub has already been released)?
> - stubs[i].valid = FALSE;

---
Jeroen Janssen

Implement StdMarshal::ReleaseMarshalData

[Mike Hearn]

On Fri, 23 Jul 2004 08:29:01 +0200, Jeroen Janssen wrote:
> > - res = IRpcStubBuffer_Release(stub);
>
> I'm not sure but, is there a hypothical race condition here when a
> thread switch takes place and someone tries to reuse the stub while
> valid is not set to TRUE yet (but the stub has already been released)?
> > - stubs[i].valid = FALSE;

You're right, good catch. I'm not sure it's worth resending the patch
though as really hardly any of our COM implementation is thread safe at
all. If Alexandre wants to reverse the two statements as he applies the
patch though he should feel welcome to :)

Implement StdMarshal::ReleaseMarshalData

[Jeroen Janssen]

Mike Hearn wrote:
> You're right, good catch. I'm not sure it's worth resending the patch
> though as really hardly any of our COM implementation is thread safe at
> all.

What exactly does this mean? (the impact on running programs that make 
use of COM)
---
Jeroen Janssen

Implement StdMarshal::ReleaseMarshalData

[Mike Hearn]

On Fri, 23 Jul 2004 14:28:20 +0200, Jeroen Janssen wrote:
> What exactly does this mean? (the impact on running programs that make 
> use of COM)

Applications that aggressively use multithreading with our DCOM code will
crash or suffer intermittent errors. Fortunately InstallShield doesn't.

I'm also not 100% convinced our COM is fully re-entrant either, but I
don't have any explicit instances in mind for that, it's just a general
feeling I have from having read the code ...

thanks -mike

Implement StdMarshal::ReleaseMarshalData

[Mike McCormack]

Mike Hearn wrote:
> You're right, good catch. I'm not sure it's worth resending the patch
> though as really hardly any of our COM implementation is thread safe at
> all.

Please don't make assertions like this.  If you make people believe that 
"code X has fault Y" without specifying exactly where the problem is, 
you'll make everybody believe that the problem is too hard to fix and 
nobody will bother.  Furthermore you'll encourage others to submit more 
code with problems, and justify it like you've just done by saying "it's 
broken anyway".

If you're aware of any place that the code is unthreadsafe or 
non-reentrant, please submit a patch fixing it or adding a comment 
saying so.

The best way to make it work right is to start with the right attitude.

Mike