Re: Implement a typelib loader cache
"Mike Hearn" <[EMAIL PROTECTED]> wrote: > Yes, it should also be path normalized, but for some reason I remember > that being more complex than I thought it'd be. > > Which function should I use here? There is no strcasecmpW, in shlwapi > there is a function which does that (StrCmpIW or something). Should I > copy/paste that into unicode.h? Just use strcmpiW instead of strcmpW. It's also declared in wine/unicode.h and exported by libwine_unicode. -- Dmitry.
Re: Implement a typelib loader cache
Mike Hearn <[EMAIL PROTECTED]> writes: > ChangeLog: > Implement a typelib loader cache You'll need to protect the list with a critical section. Also it's probably better to store the list directly in the typelib structure instead of allocating a separate object. -- Alexandre Julliard [EMAIL PROTECTED]
Re: Implement a typelib loader cache
Yes, it should also be path normalized, but for some reason I remember that being more complex than I thought it'd be. Which function should I use here? There is no strcasecmpW, in shlwapi there is a function which does that (StrCmpIW or something). Should I copy/paste that into unicode.h? On Sun, 2003-08-10 at 02:12, Dmitry Timoshkov wrote: > "Mike Hearn" <[EMAIL PROTECTED]> wrote: > > > Implement a typelib loader cache, minor debug message improvements > > + for (entry = tlb_cache_tail; entry != NULL; entry = entry->next) > + if (!strcmpW(entry->path, szPath)) { > + TRACE("cache hit\n"); > > Shouldn't this be a case insensitive comparison?
Re: Implement a typelib loader cache
"Mike Hearn" <[EMAIL PROTECTED]> wrote: > Implement a typelib loader cache, minor debug message improvements + for (entry = tlb_cache_tail; entry != NULL; entry = entry->next) + if (!strcmpW(entry->path, szPath)) { + TRACE("cache hit\n"); Shouldn't this be a case insensitive comparison? -- Dmitry.
typelib caching
Well, it works, finally. No more crashes/hangs :) That is - sort of. Unfortunately, it's a bit too slow to be usable. A few wild stabs in the dark and some timing traces showed the culprit to be LoadTypeLibEx: trace:ole:LoadTypeLibEx File L"C:\\windows\\system\\SHDOCVW.DLL" index 1 trace:ole:LoadTypeLibEx returns , in 15 milliseconds trace:ole:LoadTypeLibEx File L"C:\\windows\\system\\SHDOCVW.DLL" index 1 (h, we load it again?) trace:ole:LoadTypeLibEx returns , in 46 milliseconds (odd, slower than last time) trace:ole:LoadTypeLibEx File L"C:\\windows\\system\\mshtml.tlb" index 1 trace:ole:LoadTypeLibEx returns , in 1208 milliseconds (over a second parsing the type library? well, maybe it is quite large) trace:ole:LoadTypeLibEx File L"C:\\windows\\system\\mshtml.tlb" index 1 trace:ole:LoadTypeLibEx returns , in 6689 milliseconds (almost 7 seconds to parse the type lib a second time!?!) trace:ole:LoadTypeLibEx File L"C:\\windows\\system\\mshtml.tlb" index 1 trace:ole:LoadTypeLibEx returns , in 42139 milliseconds Waaah, 42 seconds! There's a big source of the sluggishness. So, a few questions that I can chew on tomorrow: * Shouldn't typelib loading be cached somewhere? Reparsing typelibs like this seems rather wasteful. * Why on earth would the time taken go up so massively on each run? * Where abouts in the typelib parsing could the speed hit be? Pretty clearly no typelib is going to be so complex it takes an Athlon 1200 almost a minute to parse, there is a nasty bug there somewhere. thanks -mike
Re: Typelib marshalling BSTRs
On Thu, 2003-07-24 at 13:47, Ove Kaaven wrote: > You have to use CoMarshalInterface, but of course you need an IStream > interface to do this, which is probably why I haven't bothered to > implement it yet. I implemented it by creating an HGLOBAL stream then locking the global and doing a memcpy. Not especially high performance but it works and the code is well insulated. I found the necessary flags were in the loword of pFlags. > As for the two spellings of marshal, I think the original DCE RPC > reference code (that MS-RPC was designed as a clone of) was the original > culprit, using the "marshall" spelling everywhere, and since Microsoft > adopted (embraced and extended) their API, they adopted their spelling > too. I assume the original DCE engineers were exactly that, just > engineers, not English majors (perhaps they got confused by the > unrelated noun (and name) marshall or something). Microsoft's OLE > department knew how to spell, though, so that's why we have > CoMarshalInterface in OLE, and Ndr*Marshall in MS-RPC. Hehe, yes, this sounds possible. Well, no matter, I was just a bit curious. Anyway, dispatch variant marshalling works now. On to the next crash :) thanks -mike
Re: Typelib marshalling BSTRs
ons, 23.07.2003 kl. 17.28 skrev Mike Hearn: > So, moving on from that problem, I think my woes (currently) are caused > by the NDR engine not handling VT_DISPATCH variants. Hmm. Oh yeah, I guess that could be a problem. > that causes VARIANT_UserMarshal to skip doing anything special for > VT_DISPATCH variants, when I think in reality, it should call > NdrInterfacePointerMarshall (btw, what's up with two spellings of > "marshal"?) to get the IDispatch* into the stream. You can't call NdrInterfacePointerMarshall, it's a NDR-engine function that depends on marshalling state that isn't available at this point. You have to use CoMarshalInterface, but of course you need an IStream interface to do this, which is probably why I haven't bothered to implement it yet. You can probably copy the IStream-related code from ndr_ole.c and adapt it for this situation. It may cause some code duplication to do that, but perhaps this undocumented WdtpInterfacePointer_UserMarshal mentioned elsewhere in this thread is actually a common CoMarshalInterface-wrapping back-end used by both NdrInterfacePointerMarshall and VARIANT_UserMarshal? In that case the IStream stuff would be implemented there, and there'd be no code duplication. As for the two spellings of marshal, I think the original DCE RPC reference code (that MS-RPC was designed as a clone of) was the original culprit, using the "marshall" spelling everywhere, and since Microsoft adopted (embraced and extended) their API, they adopted their spelling too. I assume the original DCE engineers were exactly that, just engineers, not English majors (perhaps they got confused by the unrelated noun (and name) marshall or something). Microsoft's OLE department knew how to spell, though, so that's why we have CoMarshalInterface in OLE, and Ndr*Marshall in MS-RPC.
Re: Typelib marshalling BSTRs
ons, 23.07.2003 kl. 11.41 skrev Mike Hearn: > On Tue, 2003-07-22 at 17:35, Gregory M. Turner wrote: > > I was kind of ignoring this as my brain is full with cabinet things ATM... > > but it sure sounds awfully wrong, doesn't it? > > Yeah. I have the feeling that some obscure rule of C is biting me on the > backside (again). If it helps, I've been thinking that perhaps the new strict-aliasing stuff in gcc 3.3 has something to do with it. I suppose you could try -fno-strict-aliasing or something; after all, the place where riid is written to is through a typecast, which strict aliasing is hateful of. Particularly in this case, as REFIID is so very const that you're probably not supposed to assign to a variable of this type at all, and you have to typecast to a non-const pointer type to be able to. With strict aliasing, the compiler will probably ignore the non-const alias and just optimize the initial NULL into any usage of the original const variable. Of course I could be way off. > > Perhaps, this really indicates some kind of problem with the loading / linking > > or the spec.c file or something like that? Does the behavior change if you > > take out the RpcTryFinally? I would guess not. > > It does not. To recap: > > REFIID riid = NULL; - fails, because assignments to it are silently > ignored > > > REFIID riid; > riid = NULL; - works, because . it confuses the gremlins? Not sure, but more likely because the compiler might actually allocate a stack slot for riid when you leave it uninitialized and then assign to it explicitly in a statement (so that the subsequent typecast will return a pointer to the same memory). With the former initialization, the compiler probably considers the riid having a predefined value (that "won't ever change" because of REFIID's constness), and hence doesn't reserve a stack slot for it; it just uses NULL wherever riid is used (and with strict aliasing, maybe just allocates temporary space for the typecasting, not a real stack slot for riid). Well, that's my theory anyway. I probably don't have your problem anyway because my own MIDL-generated oaidl_p.c uses an explicit assignment like in your latter case. Alexandre probably just changed it into the former form to avoid some compiler warnings (from assigning to a const variable, probably).
RE: Typelib marshalling BSTRs
On Thu, 2003-07-24 at 03:38, Robert Shearman wrote: > Sorry, just realised that this the VARIANT_UserMarshal function is in > oleaut32. On Windows, this calls WdtpInterfacePointer_UserMarshal for both > the VT_UNKNOWN and VT_DISPATCH cases, which in turn calls > CoMarshalInterface. Thanks, I didn't know about those functions. Unfortunately I can't see the prototypes anywhere, and don't really have time right now to disassemble native code to find out. I'll have a look today and see what the best way of dealing with this situation might be
RE: Typelib marshalling BSTRs
On Thu, 2003-07-24 at 09:40, E Lea wrote: > Unless you don't like beer you'll probably find you won't have nearly as > much time for hacking code when you get to Uni as you'd think, or want, or > like I don't drink ;) On the other hand, there are plenty of other things that I do like doing which I'm sure will suck time. No matter, I have three years :) Plus the first year of the course is largely "learn java", which fortunately I already know.
RE: Typelib marshalling BSTRs
> > Sounds like you are doing a great job, BTW! I know this stuff gets kinda > > frustrating & tedious, but remember that you are in territory that has been a > > problem for wine since time immemorial. > > Thanks. I'm hoping that when I've got a grip on all this and settled > into uni I'll be able to write some more docs so in future people don't > have to spam the list to learn Wine OLE hacking. Unless you don't like beer you'll probably find you won't have nearly as much time for hacking code when you get to Uni as you'd think, or want, or like
RE: Typelib marshalling BSTRs
> > -Original Message- > > From: Mike Hearn [mailto:[EMAIL PROTECTED] > > Sent: 23 July 2003 23:09 > > To: Robert Shearman > > Cc: [EMAIL PROTECTED] > > Subject: Re: Typelib marshalling BSTRs > > > > Well, the code doesn't actually marshal IWebBrowser2, it operates > > entirely in terms of IDispatch interfaces. Once the dispatch interface > > for the WebBrowser control is marshalled into another thread, the code > > starts poking it via Invoke, in this case retrieving the Document > > property which also returns a dispatch interface. Sorry, just realised that this the VARIANT_UserMarshal function is in oleaut32. On Windows, this calls WdtpInterfacePointer_UserMarshal for both the VT_UNKNOWN and VT_DISPATCH cases, which in turn calls CoMarshalInterface. I hope this helps, Rob
Re: Typelib marshalling BSTRs
On Wed, 2003-07-23 at 23:04, Robert Shearman wrote: > I don't know about NdrInterfacePointerMarshall, but I don't think it should > get there to start with (see below). BTW, I think the two spellings depends > on whether the word is used as a verb or as a noun. Yeah, I considered that, but MSDN seems to use marshal as a verb as well. *shrug* > I think the code > should not try to marshal/unmarshal the data in rpcrt4, but should use the > PSOAInterface code in oleaut32 (according to the registry entry for > Interfaces\[CLSID of IWebBrowser2]\ProxyStubClsid32). It would probably be > best to find out why this is not happening (maybe by moving some oleaut32 > code to rpcrt4?). Well, the code doesn't actually marshal IWebBrowser2, it operates entirely in terms of IDispatch interfaces. Once the dispatch interface for the WebBrowser control is marshalled into another thread, the code starts poking it via Invoke, in this case retrieving the Document property which also returns a dispatch interface. I'm pretty sure just marshalling IWebBrowser2 would work, but unfortunately the nature of the Java-COM bridge we use (neva coroutine) means that it all goes via IDispatch. That, in turn, means we have to support marshalling of VT_DISPATCH variants. thanks -mike
Re: Typelib marshalling BSTRs
> Subject: Re: Typelib marshalling BSTRs > From: Mike Hearn <[EMAIL PROTECTED]> > To: "Gregory M. Turner" <[EMAIL PROTECTED]> > Cc: [EMAIL PROTECTED] > Date: 23 Jul 2003 16:28:58 +0100 > > So, moving on from that problem, I think my woes (currently) are caused > by the NDR engine not handling VT_DISPATCH variants. > > in wire_size(): > case VT_DISPATCH: > FIXME("wire-size interfaces\n"); > > that causes VARIANT_UserMarshal to skip doing anything special for > VT_DISPATCH variants, when I think in reality, it should call > NdrInterfacePointerMarshall (btw, what's up with two spellings of > "marshal"?) to get the IDispatch* into the stream. I don't know about NdrInterfacePointerMarshall, but I don't think it should get there to start with (see below). BTW, I think the two spellings depends on whether the word is used as a verb or as a noun. > While rather roundabout, I think it seems clear that this is what's > needed. The problem then becomes one of NdrInterfacePointerMarshall() > needing information that isn't available from inside > VARIANT_UserMarshal, like the MIDL_STUB_BUFFER pointer. > > Greg, as you are the relevant Guru here and Ove is probably a bit pissed > off with all my questions, what should we be doing here? If we are meant > to use NdrInterfacePointerMarshall to handle VT_DISPATCH variants, how > do we get the args it needs? Ok, my name's not Greg or Ove, but I'll have a go anyway. I think the code should not try to marshal/unmarshal the data in rpcrt4, but should use the PSOAInterface code in oleaut32 (according to the registry entry for Interfaces\[CLSID of IWebBrowser2]\ProxyStubClsid32). It would probably be best to find out why this is not happening (maybe by moving some oleaut32 code to rpcrt4?). Rob
Re: Typelib marshalling BSTRs
On Wed, 2003-07-23 at 20:35, Gregory M. Turner wrote: > Tell me abou it... both avenues are pretty darn thorny! My hope is that if I > implement the LPC 'ports' api, NT rpcrt4/ole32/oleaut32 combo's will show > more promise... but of course there is no guarantee until I try, which is > going to take a while... you are probably on the right course imo, and > certainly the course most likely to be helpful to wine. The win98 versions are still available and work pretty well, the only big problem being that obviously it's not free software, with all the debugging and redistribution nastyness that implies. > Sounds like you are doing a great job, BTW! I know this stuff gets kinda > frustrating & tedious, but remember that you are in territory that has been a > problem for wine since time immemorial. Thanks. I'm hoping that when I've got a grip on all this and settled into uni I'll be able to write some more docs so in future people don't have to spam the list to learn Wine OLE hacking. > 10-4. It just caught my eye as a red-flag for some deeper problem that may > deserve longer-term attention... Yes, it's definately worth remembering. A friend suggested compiler bug, which is possible, but then isn't gcc invincible? ;) > > The problem I face now is that the property get request returns the HTML > > DOM IDispatch in a variant of type VT_DISPATCH, which the Ndr variant > > marshalling code doesn't seem able to handle. > > unfortunately, this oleaut stuff is largely unknown to me... is this using > CoMarshallInterface? I assume I'd need to call that in order to marshal it, yes. > If it's in CoMarshallInterface, I think you will just create an infinite loop. > It appears that CoMarshallInterface is called by NdrInterfacePointerMarshall > to do all the dirty work. I don't think I'll create an infinite loop, AFAIK the interface marshalling code doesn't use the NDR engine, it just writes to an IStream. In fact the ndr_ole code wraps the NDR buffer in a simple IStream implementation so that API can be used. It's just a case of having the right data at hand. > Sorry, I wish I was a little more up on this stuff, but a lot of code has gone > into rpcrt4 marshalling that I haven't even taken the time to audit and > properly understand yet, and once we leave the domain of pure RPC and start > mixing in with OLE & OLE automation I'm pretty ignorant... I've been meaning > to correct that, but so far other things have occupied my time. Thanks for the encouragement anyway :) I'll think of something. by the end of it I'm going to have a pretty good overview of most areas of OLE. thanks -mike
Re: Typelib marshalling BSTRs
On Wednesday 23 July 2003 12:03 pm, Mike Hearn wrote: > One dependency that isn't simple is that it embeds Internet Explorer and > uses it as an HTML enabled word processor. The app uses a Java<->COM > bridge to do that, and as such gives OLE quite a work out. Originally, > not having much time, I just used microsofts OLE, which worked pretty > well but unfortunately there was a problem with it deadlocking on thread > detach, which blocked the loader section and so everything died. After > trying to divine what was going wrong there for about a week, I decided > that it was fruitless and I'd be better bringing Wines DCOM/OLE up to > scratch. Tell me abou it... both avenues are pretty darn thorny! My hope is that if I implement the LPC 'ports' api, NT rpcrt4/ole32/oleaut32 combo's will show more promise... but of course there is no guarantee until I try, which is going to take a while... you are probably on the right course imo, and certainly the course most likely to be helpful to wine. > Because this app does many horrid things, like marshalling IDispatch > between threads in order to make calls from Java to IE (C++), I needed > Oves patch to add these features, which he kindly sent. I've been > hacking on that ever since, trying to make it able to do all the things > said app needs. Cue wild learning spree as I try to cram the entirety of > COM into my head in a few days :) Sounds like you are doing a great job, BTW! I know this stuff gets kinda frustrating & tedious, but remember that you are in territory that has been a problem for wine since time immemorial. > The issue that plagued me before (which i've now hacked around) was that > the call was segfaulting inside IE, because riid == NULL, not IID_NULL. > Clearly this field is used, even though it's marked as reserved by > Microsoft, as if it's wrong IE dereferences it and dies. great find. > The reason NULL > was being passed in instead of IID_NULL was because in the MIDL > generated stubs, the Ndr call which demarshalls the iid was passed a > pointer to a local variable (&riid), initialized on the stack as "RIID > riid = NULL", it set *ppMemory = demarshaled iid, and trace statements > confirmed that just before it returned *ppMemory was indeed pointing at > the right thing. Unfortunately, for some reason riid stayed at NULL. > > Changing the way riid was declared fixed that. I don't know why, and > although I'm curious to find out, I'm running out of time before my year > at QinetiQ is up and I drop the project and go back home, so I'm not > that curious :( 10-4. It just caught my eye as a red-flag for some deeper problem that may deserve longer-term attention... > The problem I face now is that the property get request returns the HTML > DOM IDispatch in a variant of type VT_DISPATCH, which the Ndr variant > marshalling code doesn't seem able to handle. unfortunately, this oleaut stuff is largely unknown to me... is this using CoMarshallInterface? > I poked about, saw that it couldn't handle that, and thought "I have a > cunning plan - why not use NdrInterfacePointerMarshall to implement this > code?", but unfortunately that function needs information that has been > lost (ie not passed through the function calls) by the time we find out > that the variant is a VT_DISPATCH. > > Now I'm stuck :( I don't know how to get the information > NdrInterfacePointerMarshall needs, namely the MIDL_STUB_BUFFER and > format string, and I can't extend the function prototypes to contain it > because they are invoked from some kind of table. Obviously hacks like > global variables are wrong. That's where I left off todays > adventures > > any insights welcome -mike If it's in CoMarshallInterface, I think you will just create an infinite loop. It appears that CoMarshallInterface is called by NdrInterfacePointerMarshall to do all the dirty work. Sorry, I wish I was a little more up on this stuff, but a lot of code has gone into rpcrt4 marshalling that I haven't even taken the time to audit and properly understand yet, and once we leave the domain of pure RPC and start mixing in with OLE & OLE automation I'm pretty ignorant... I've been meaning to correct that, but so far other things have occupied my time. -- "...he that hath no sword, let him sell his garment, and buy one." - Jesus Christ, Luke 22:36 gmt
Re: Typelib marshalling BSTRs
On Wed, 2003-07-23 at 17:50, Gregory M. Turner wrote: > Mike, somehow in this thread, I have lost track of the basic parameters of > your situation... could you remind us of the precise context in which your > code is being compiled and run? The story is like so: I have an app. Said app is written in Java, yet has many Win32 dependencies. Some of them are simple, for instance on a tray icon. Some are not. As such, I run the Sun JVM with this app under Wine. Rather ugly solution, but unfortunately there weren't really any alternatives given the context of this job. One dependency that isn't simple is that it embeds Internet Explorer and uses it as an HTML enabled word processor. The app uses a Java<->COM bridge to do that, and as such gives OLE quite a work out. Originally, not having much time, I just used microsofts OLE, which worked pretty well but unfortunately there was a problem with it deadlocking on thread detach, which blocked the loader section and so everything died. After trying to divine what was going wrong there for about a week, I decided that it was fruitless and I'd be better bringing Wines DCOM/OLE up to scratch. Because this app does many horrid things, like marshalling IDispatch between threads in order to make calls from Java to IE (C++), I needed Oves patch to add these features, which he kindly sent. I've been hacking on that ever since, trying to make it able to do all the things said app needs. Cue wild learning spree as I try to cram the entirety of COM into my head in a few days :) The place I'm at now, is that IDispatch and its methods aren't always being marshalled correctly. While IDispatch itself is now properly marshalled using the MIDL generated proxy/stub sets, one of the things this app does is manipulate the DOM from other threads, and it all goes via late bound IDispatch (because it's called from java via a generic bridge). When IDispatch::Invoke is called with a property get request, said request is dealt with succesfully by Internet Explorer, but the output (an IDispatch for the HTML DOM object) doesn't make it back to the program, which prompty throws an exception and isn't very happy. The issue that plagued me before (which i've now hacked around) was that the call was segfaulting inside IE, because riid == NULL, not IID_NULL. Clearly this field is used, even though it's marked as reserved by Microsoft, as if it's wrong IE dereferences it and dies. The reason NULL was being passed in instead of IID_NULL was because in the MIDL generated stubs, the Ndr call which demarshalls the iid was passed a pointer to a local variable (&riid), initialized on the stack as "RIID riid = NULL", it set *ppMemory = demarshaled iid, and trace statements confirmed that just before it returned *ppMemory was indeed pointing at the right thing. Unfortunately, for some reason riid stayed at NULL. Changing the way riid was declared fixed that. I don't know why, and although I'm curious to find out, I'm running out of time before my year at QinetiQ is up and I drop the project and go back home, so I'm not that curious :( The problem I face now is that the property get request returns the HTML DOM IDispatch in a variant of type VT_DISPATCH, which the Ndr variant marshalling code doesn't seem able to handle. I poked about, saw that it couldn't handle that, and thought "I have a cunning plan - why not use NdrInterfacePointerMarshall to implement this code?", but unfortunately that function needs information that has been lost (ie not passed through the function calls) by the time we find out that the variant is a VT_DISPATCH. Now I'm stuck :( I don't know how to get the information NdrInterfacePointerMarshall needs, namely the MIDL_STUB_BUFFER and format string, and I can't extend the function prototypes to contain it because they are invoked from some kind of table. Obviously hacks like global variables are wrong. That's where I left off todays adventures any insights welcome -mike
Re: Typelib marshalling BSTRs
On Wednesday 23 July 2003 06:17 am, Mike Hearn wrote: > On Wed, 2003-07-23 at 11:15, Michael Stefaniuc wrote: > > If > > yes then the gcc is probably throwing the variable away. Did you tried > > to compile it with -O0 or just put a "volatile" in front of it: > > volatile REFIID riid = NULL; > > I tried using volatile, no cigar. Mike, somehow in this thread, I have lost track of the basic parameters of your situation... could you remind us of the precise context in which your code is being compiled and run? I.e.: is this in a wine dll, a standalone exe, inside or outside the wine source tree, how is your code invoked, etc? Not that I expect such information will enable me to solve your problem :P But obviously it behooves us to at least determine when and where this problem is manifest, so that we may avoid it or that Alexandre may fix it ;) -- "...he that hath no sword, let him sell his garment, and buy one." - Jesus Christ, Luke 22:36 gmt
Re: Typelib marshalling BSTRs
So, moving on from that problem, I think my woes (currently) are caused by the NDR engine not handling VT_DISPATCH variants. in wire_size(): case VT_DISPATCH: FIXME("wire-size interfaces\n"); that causes VARIANT_UserMarshal to skip doing anything special for VT_DISPATCH variants, when I think in reality, it should call NdrInterfacePointerMarshall (btw, what's up with two spellings of "marshal"?) to get the IDispatch* into the stream. While rather roundabout, I think it seems clear that this is what's needed. The problem then becomes one of NdrInterfacePointerMarshall() needing information that isn't available from inside VARIANT_UserMarshal, like the MIDL_STUB_BUFFER pointer. Greg, as you are the relevant Guru here and Ove is probably a bit pissed off with all my questions, what should we be doing here? If we are meant to use NdrInterfacePointerMarshall to handle VT_DISPATCH variants, how do we get the args it needs? thanks -mike
Re: Typelib marshalling BSTRs
On Wed, 2003-07-23 at 11:15, Michael Stefaniuc wrote: > If > yes then the gcc is probably throwing the variable away. Did you tried > to compile it with -O0 or just put a "volatile" in front of it: > volatile REFIID riid = NULL; I tried using volatile, no cigar.
Re: Typelib marshalling BSTRs
Hello, On Wed, Jul 23, 2003 at 10:41:12AM +0100, Mike Hearn wrote: > > Perhaps, this really indicates some kind of problem with the loading / linking > > or the spec.c file or something like that? Does the behavior change if you > > take out the RpcTryFinally? I would guess not. > > It does not. To recap: > > REFIID riid = NULL; - fails, because assignments to it are silently > ignored > > > REFIID riid; > riid = NULL; - works, because . it confuses the gremlins? Is this in a callback function and riid isn't used in the function? If yes then the gcc is probably throwing the variable away. Did you tried to compile it with -O0 or just put a "volatile" in front of it: volatile REFIID riid = NULL; bye michael -- Michael Stefaniuc Tel.: +49-711-96437-199 System Administration Fax.: +49-711-96437-111 Red Hat GmbHEmail: [EMAIL PROTECTED] Hauptstaetterstr. 58http://www.redhat.de/ D-70178 Stuttgart pgp0.pgp Description: PGP signature
Re: Typelib marshalling BSTRs
On Tue, 2003-07-22 at 17:35, Gregory M. Turner wrote: > I was kind of ignoring this as my brain is full with cabinet things ATM... > but it sure sounds awfully wrong, doesn't it? Yeah. I have the feeling that some obscure rule of C is biting me on the backside (again). > ./include/rpc.h-54-/* ignore exception handling for now */ > ./include/rpc.h-55-#define RpcTryExcept if (1) { > ./include/rpc.h-56-#define RpcExcept(expr) } else { > ./include/rpc.h-57-#define RpcEndExcept } > ./include/rpc.h:58:#define RpcTryFinally > ./include/rpc.h-59-#define RpcFinally > ./include/rpc.h-60-#define RpcEndFinally > ./include/rpc.h-61-#define RpcExceptionCode() 0 > > Considering the implementation, this seems like an awfully bizarre behavior. Oops :) I saw all the fascinating code you were throwing around earlier for SEH and assumed that the existing code looked a bit like it. Clearly not. > Perhaps, this really indicates some kind of problem with the loading / linking > or the spec.c file or something like that? Does the behavior change if you > take out the RpcTryFinally? I would guess not. It does not. To recap: REFIID riid = NULL; - fails, because assignments to it are silently ignored REFIID riid; riid = NULL; - works, because . it confuses the gremlins?
Re: Typelib marshalling BSTRs
On Tuesday 22 July 2003 06:25 am, Mike Hearn wrote: > On Mon, 2003-07-21 at 17:30, Ove Kaaven wrote: > > > I found that it's in IDispatch_Invoke_Stub - it calls IDispatch::Invoke > > > on the actual object and crashes, in native shdocvw code. So, I guess > > > the inputs its given there are incorrect somehow. I'll look into it > > > some more. > > > > OK. > > I found the problem was due to the location of the riid = NULL > initializer code. For some reason, when the: REFIID riid = NULL; line is > at the function prologue, assigning to it using *ppMemory doesn't work. > If the variable is initialized to NULL inside the RPC TryFinally area, > just before the call to NdrSimpleStructUnmarshall, it works OK. > > So, I'm CCing Greg as well, to see if he knows why the Wine SEH code > might interfere with setting the values like that. As it is, until the > actual problem is found I just have to remember to edit the midl output. I was kind of ignoring this as my brain is full with cabinet things ATM... but it sure sounds awfully wrong, doesn't it? ./include/rpc.h-54-/* ignore exception handling for now */ ./include/rpc.h-55-#define RpcTryExcept if (1) { ./include/rpc.h-56-#define RpcExcept(expr) } else { ./include/rpc.h-57-#define RpcEndExcept } ./include/rpc.h:58:#define RpcTryFinally ./include/rpc.h-59-#define RpcFinally ./include/rpc.h-60-#define RpcEndFinally ./include/rpc.h-61-#define RpcExceptionCode() 0 Considering the implementation, this seems like an awfully bizarre behavior. Perhaps, this really indicates some kind of problem with the loading / linking or the spec.c file or something like that? Does the behavior change if you take out the RpcTryFinally? I would guess not. -gmt -- "...he that hath no sword, let him sell his garment, and buy one." - Jesus Christ, Luke 22:36 gmt
Re: Typelib marshalling BSTRs
On Mon, 2003-07-21 at 17:30, Ove Kaaven wrote: > > I found that it's in IDispatch_Invoke_Stub - it calls IDispatch::Invoke > > on the actual object and crashes, in native shdocvw code. So, I guess > > the inputs its given there are incorrect somehow. I'll look into it some > > more. > > OK. I found the problem was due to the location of the riid = NULL initializer code. For some reason, when the: REFIID riid = NULL; line is at the function prologue, assigning to it using *ppMemory doesn't work. If the variable is initialized to NULL inside the RPC TryFinally area, just before the call to NdrSimpleStructUnmarshall, it works OK. So, I'm CCing Greg as well, to see if he knows why the Wine SEH code might interfere with setting the values like that. As it is, until the actual problem is found I just have to remember to edit the midl output. thanks -mike
Re: Typelib marshalling BSTRs
man, 21.07.2003 kl. 17.31 skrev Mike Hearn: > On Mon, 2003-07-21 at 12:55, Ove Kaaven wrote: > > > fixme:ole:PointerUnmarshall unhandled ptr type=12 > > > > This means that null pointers may not be handled properly (but non-null > > pointers will still work). However, I have yet to see a situation where > > I needed to handle it, but then again I've only worked on InstallShield. > > I suppose it's possible that you need to if you're working on something > > else (but I don't think IDispatch is an interface where you need to > > worry about it). > > IDispatch::Invoke has an riid parameter, that the docs say is reserved > and must be IID_NULL. In the current custom marshallers, trace > statements reveal that the stub receives this as NULL, not IID_NULL. Are > they equivalent in this context, or is that being marshalled > incorrectly? Not exactly equivalent here. The ptr type is not 0x12 here, I think; supplying NULL here is completely invalid and the MIDL code in oaidl_p.c will raise an exception, completely refusing to marshal the call at all. If supplying NULL is supposed to be valid at the API level, it must be handled by the IDispatch_Invoke_Proxy wrapper in usrmarshal.c, converting the NULL to IID_NULL, before passing it off to the MIDL-generated marshaller. (This Invoke wrapper already treats some other can-be-NULL arguments in this manner, so this would just be yet another case of this.) > > You probably just need to marshal an extra flag saying > > whether the pointer is null or not, but since I haven't seen the DCE RPC > > spec I don't know what the flag should look like. > > Is that just a case of adding another IStream::Write call, and having > the equivalent read in the custom demarshaller? That would probably work, except that MIDL-based marshallers and rpcrt4 don't use IStream... > > Hmm. It could still be in Wine code, since NdrComplexArrayUnmarshall > > might be trying to unmarshal an array of variants, which is a custom > > type. The custom-marshalling code for variants is in > > dlls/oleaut32/usrmarshal.c, and there is a VariantInit in the > > VARIANT_UserUnmarshal function in there (though it probably shouldn't > > have shown up in a relay trace). > > I found that it's in IDispatch_Invoke_Stub - it calls IDispatch::Invoke > on the actual object and crashes, in native shdocvw code. So, I guess > the inputs its given there are incorrect somehow. I'll look into it some > more. OK.
Re: Typelib marshalling BSTRs
On Sun, 2003-07-20 at 12:29, Ove Kaaven wrote: > IDispatch should not be marshalled by the typelib marshaller, the method > arguments it use cannot be represented by a typelib, as you've seen. The > MIDL code in oaidl_p.c must be used instead. You need to change the > registry to use the right marshaller, I think the necessary > winedefault.reg changes were included in my patch, but I'm not sure. I'd just like to double check that it *is* actually possible to marshal IDispatch, right? I've got it using the marshallers in oaidl_p.c, but I get fixmes from the RPC runtime like so: fixme:ole:NdrConvert (pStubMsg == ^0x4d7a27a4, pFormat == ^0x53bc5f84): stub. fixme:ole:PointerUnmarshall unhandled ptr type=12 and so on. Eventually it crashes inside native code (i think) just after returning from the proxy (well, i can see the last call to NdrComplexArrayUnmarshall) and calling VariantInit. Are those fixmes actually critical? The NdrConvert stub at least suggests that it might be, and that it should throw an exception. any insights welcome, thanks -mike
Re: Typelib marshalling BSTRs
On Sun, 2003-07-20 at 12:56, Ove Kaaven wrote: > I've mentioned before the InstallShield hack in typelib.c (the "if (1 || > ...)"), right? I suspect that you *must* disable it. or it'll probably > keep reinstalling the wrong marshaller into the registry every time you > run it. Eek, I didn't realise that hack would actually modify the registry on each run, I thought it just diverted something onto a different codepath. I'll try disabling that as well. thanks -mike
Re: Typelib marshalling BSTRs
s?, 20.07.2003 kl. 13.38 skrev Mike Hearn: > > Sorry for the late answer, I've been busy attending debconf3... > > Ah, no problem, I hope it was good :) Not bad. > > IDispatch should not be marshalled by the typelib marshaller, the method > > arguments it use cannot be represented by a typelib, as you've seen. The > > MIDL code in oaidl_p.c must be used instead. You need to change the > > registry to use the right marshaller, I think the necessary > > winedefault.reg changes were included in my patch, but I'm not sure. > > There were some changes yes, it seems for some reason they are not being > used. I will double check. I probably simply forgot to remerge the > default registry, thinking about it. My mind wasn't very clear on > anything by friday :) I've mentioned before the InstallShield hack in typelib.c (the "if (1 || ...)"), right? I suspect that you *must* disable it. or it'll probably keep reinstalling the wrong marshaller into the registry every time you run it. > In Delphi/Object Pascal passing an array like that would perform a copy > by value, not pass the address. I (wrongly) assumed C would have similar > rules. This is what I get for learning it as I go :) Hmm, OK.
Re: Typelib marshalling BSTRs
> Sorry for the late answer, I've been busy attending debconf3... Ah, no problem, I hope it was good :) > IDispatch should not be marshalled by the typelib marshaller, the method > arguments it use cannot be represented by a typelib, as you've seen. The > MIDL code in oaidl_p.c must be used instead. You need to change the > registry to use the right marshaller, I think the necessary > winedefault.reg changes were included in my patch, but I'm not sure. There were some changes yes, it seems for some reason they are not being used. I will double check. I probably simply forgot to remerge the default registry, thinking about it. My mind wasn't very clear on anything by friday :) > That should be the same thing. In expressions, the compiler will treat > an array variable as a pointer to its first element. This feature has > always been in C (and have been useful for no ends of code obfuscation > tricks, as well as numerous novice-programmer bugs). In my opinion, > &args[0] is more unredable than just using args, so the latter is my > preferred style. But I'm not sure what you thought it meant? In Delphi/Object Pascal passing an array like that would perform a copy by value, not pass the address. I (wrongly) assumed C would have similar rules. This is what I get for learning it as I go :) thanks -mike
Re: Typelib marshalling BSTRs
fre, 18.07.2003 kl. 17.53 skrev Mike Hearn: > Well, I've made pretty good progress, but am a bit stuck with this > problem. Basically: > > 003d:trace:ole:TL_Marshal parameter 1 > 003d:trace:ole:TL_Marshalname : L"rgszNames" > 003d:trace:ole:TL_Marshaltype : 26 > 003d:trace:ole:TL_Marshalflags : 01 > 003d:trace:ole:TL_MarshalTypedereferencing PTR 0x52edf7d0 => > 0x54331260 > 003d:trace:ole:TL_MarshalTypedereferencing PTR 0x54331260 => > 0x54331270 > 003d:trace:ole:TL_MarshalTypemarshaling byte 100 > > As you can see, it's marshalling IDispatch across apartment boundaries, > but it gets this parameter wrong. The double dereference seems correct, > but marshalling a byte is not - I think it should be a BSTR. > > rgszNames is a parameter of IDispatch::GetIDsFromNames, which is defined > as an OLECHAR FAR* FAR* Sorry for the late answer, I've been busy attending debconf3... IDispatch should not be marshalled by the typelib marshaller, the method arguments it use cannot be represented by a typelib, as you've seen. The MIDL code in oaidl_p.c must be used instead. You need to change the registry to use the right marshaller, I think the necessary winedefault.reg changes were included in my patch, but I'm not sure. > I can't figure out where the typeinfo data for IDispatch is coming from > though, hence my stuckness. stdole32.tlb > BTW, is this line really correct: > > TL_Unmarshal(pStm, pInfo, pDesc, PARAMFLAG_FIN, args, &argc, pChannel, > &is_iid); > > shouldn't it be: > > TL_Unmarshal(pStm, pInfo, pDesc, PARAMFLAG_FIN, (DWORD*)&args[0], &argc, > pChannel, &is_iid); That should be the same thing. In expressions, the compiler will treat an array variable as a pointer to its first element. This feature has always been in C (and have been useful for no ends of code obfuscation tricks, as well as numerous novice-programmer bugs). In my opinion, &args[0] is more unredable than just using args, so the latter is my preferred style. But I'm not sure what you thought it meant?
RE: Typelib marshalling BSTRs
> -Original Message- > From: Kelly Leahy [mailto:[EMAIL PROTECTED] > Sent: 18 July 2003 21:44 > To: Mike Hearn; Robert Shearman > Cc: [EMAIL PROTECTED] > Subject: Re: Typelib marshalling BSTRs > > > How does typelib marshalling handle arrays? It's marshalling IDispatch, > > and this parameter is in IDispatch::GetIDsFromNames(). > > > my guess would be that IDispatch has a custom marshaller defined > in windows. > You should be able to verify this in the registry (though I don't remember > how). > It already has a custom marshaller (in dlls/oleaut32/oaidl_p.c). The question is: should it be called from here? I would guess that the VT_PTR handling is a incomplete at the moment, since it appears to only be marshalling the one byte, whereas it should be marshalling an array of bytes. To be honest, I don't think the VT_PTR case can marshal anything correctly, without calling the associated proxy/stub function (custom marshaller). I probably don't understand enough to answer this question and Ove is probably the best person to answer this, but I thought I'd give it stab anyway and see if this makes sense. Rob
Re: Typelib marshalling BSTRs
> > There is no way of expressing size_of once the IDL is in TypeLib form. It is > > effectively ignored. However, it is used in the proxy and stub functions > > generated. I'm not sure what interface it is marshalling here, but your > > error would appear to be elsewhere. > > How does typelib marshalling handle arrays? It's marshalling IDispatch, > and this parameter is in IDispatch::GetIDsFromNames(). > my guess would be that IDispatch has a custom marshaller defined in windows. You should be able to verify this in the registry (though I don't remember how). Kelly
Re: Typelib marshalling BSTRs
> There is no way of expressing size_of once the IDL is in TypeLib form. It is > effectively ignored. However, it is used in the proxy and stub functions > generated. I'm not sure what interface it is marshalling here, but your > error would appear to be elsewhere. How does typelib marshalling handle arrays? It's marshalling IDispatch, and this parameter is in IDispatch::GetIDsFromNames(). > Using a test program which dumps type libraries, I can confirm that LPOLESTR > * is compiled to CHAR** on Windows. I can't quite seem why, but it appears > that the marshalling that is being done is correct, at least for the > information available to it. Well, the invocation returns DISP_E_UNKNOWNNAME, which is a valid return value for the function, but as the call is successful when using native ole dlls the most obvious possibility is that the names array is being incorrectly marshalled (which would lead to no matches being found). thanks -mike
Re: Typelib marshalling BSTRs
> Subject: Re: Typelib marshalling BSTRs > From: Mike Hearn <[EMAIL PROTECTED]> > To: Kelly Leahy <[EMAIL PROTECTED]> > Cc: [EMAIL PROTECTED] > Date: 18 Jul 2003 17:24:07 +0100 > > On Fri, 2003-07-18 at 17:08, Kelly Leahy wrote: > > weird. are the "type" and "flags" correct? > > They appear to be. Type 26 is VT_PTR, flags 1 == in param > > > bstr isn't right, since it's an array of bstr (or more > precisely array of > > OLECHAR FAR* - which may also be causing problems) - though I > may be wrong > > if it marshals them one by one. > > Yeah, I had a thinko. It's supposed to be an array of bstr, but even > simply marshalling the first element correctly would be good :) No, it is supposed to be OLECHAR which is different (in terms of VARIANT types) to BSTR. BSTR implies a size field at the start of the array. > HRESULT GetIDsOfNames( > [in] REFIID riid, > [in, size_is(cNames)] LPOLESTR *rgszNames, > [in] UINT cNames, > [in] LCID lcid, > [out, size_is(cNames)] DISPID *rgDispId); > > Maybe the size_is() field should be noticed somewhere in the marshalling > code. I don't know where such information would surface though, it's not > in the flags, there is no PARAMFLAG constant for it. There is no way of expressing size_of once the IDL is in TypeLib form. It is effectively ignored. However, it is used in the proxy and stub functions generated. I'm not sure what interface it is marshalling here, but your error would appear to be elsewhere. Using a test program which dumps type libraries, I can confirm that LPOLESTR * is compiled to CHAR** on Windows. I can't quite seem why, but it appears that the marshalling that is being done is correct, at least for the information available to it. Rob
Re: Typelib marshalling BSTRs
On 18 Jul 2003, Mike Hearn wrote: > Hm, so if you declare an array like so: > > DWORD args[32]; > > and then pass args as a parameter of type "DWORD *args", it > automatically gets the address of it? I thought passing variables > directly like that would normally cause a copy by value, but I don't > know C well enough I suppose. Yes, in C everything is passed by value, but args is a pointer to 32 consecutive DWORDs, so in this case the value of args is the address, and it's the address that gets copied. -- Dimi.
Re: Typelib marshalling BSTRs
> Hm, so if you declare an array like so: > > DWORD args[32]; > > and then pass args as a parameter of type "DWORD *args", it > automatically gets the address of it? I thought passing variables > directly like that would normally cause a copy by value, but I don't > know C well enough I suppose. > yes. As far as parameter passing goes, xxx *bob is the same as xxx bob[]. this is always true in C and C++, as far as I can remember. Kelly
Re: Typelib marshalling BSTRs
On Fri, 2003-07-18 at 17:08, Kelly Leahy wrote: > weird. are the "type" and "flags" correct? They appear to be. Type 26 is VT_PTR, flags 1 == in param > bstr isn't right, since it's an array of bstr (or more precisely array of > OLECHAR FAR* - which may also be causing problems) - though I may be wrong > if it marshals them one by one. Yeah, I had a thinko. It's supposed to be an array of bstr, but even simply marshalling the first element correctly would be good :) > Shouldn't there be some special attributes > in the IDL for this stuff, like size_of() or something like that (I'm a bit > rusty)? HRESULT GetIDsOfNames( [in] REFIID riid, [in, size_is(cNames)] LPOLESTR *rgszNames, [in] UINT cNames, [in] LCID lcid, [out, size_is(cNames)] DISPID *rgDispId); Maybe the size_is() field should be noticed somewhere in the marshalling code. I don't know where such information would surface though, it's not in the flags, there is no PARAMFLAG constant for it. > these should be identical in terms of the code that gets compiles, though > the second form may eliminate some warnings by using the typecast. > Incidentally, you could just use (DWORD*)args to have the same effect. Hm, so if you declare an array like so: DWORD args[32]; and then pass args as a parameter of type "DWORD *args", it automatically gets the address of it? I thought passing variables directly like that would normally cause a copy by value, but I don't know C well enough I suppose.
Re: Typelib marshalling BSTRs
> Hi, > > Well, I've made pretty good progress, but am a bit stuck with this > problem. Basically: > > 003d:trace:ole:TL_Marshal parameter 1 > 003d:trace:ole:TL_Marshalname : L"rgszNames" > 003d:trace:ole:TL_Marshaltype : 26 > 003d:trace:ole:TL_Marshalflags : 01 > 003d:trace:ole:TL_MarshalTypedereferencing PTR 0x52edf7d0 => > 0x54331260 > 003d:trace:ole:TL_MarshalTypedereferencing PTR 0x54331260 => > 0x54331270 > 003d:trace:ole:TL_MarshalTypemarshaling byte 100 > weird. are the "type" and "flags" correct? > As you can see, it's marshalling IDispatch across apartment boundaries, > but it gets this parameter wrong. The double dereference seems correct, > but marshalling a byte is not - I think it should be a BSTR. > > rgszNames is a parameter of IDispatch::GetIDsFromNames, which is defined > as an OLECHAR FAR* FAR* > bstr isn't right, since it's an array of bstr (or more precisely array of OLECHAR FAR* - which may also be causing problems) - though I may be wrong if it marshals them one by one. Shouldn't there be some special attributes in the IDL for this stuff, like size_of() or something like that (I'm a bit rusty)? > I can't figure out where the typeinfo data for IDispatch is coming from > though, hence my stuckness. BTW, is this line really correct: > > TL_Unmarshal(pStm, pInfo, pDesc, PARAMFLAG_FIN, args, &argc, pChannel, > &is_iid); > > shouldn't it be: > > TL_Unmarshal(pStm, pInfo, pDesc, PARAMFLAG_FIN, (DWORD*)&args[0], &argc, > pChannel, &is_iid); > these should be identical in terms of the code that gets compiles, though the second form may eliminate some warnings by using the typecast. Incidentally, you could just use (DWORD*)args to have the same effect. Kelly
Typelib marshalling BSTRs
Hi, Well, I've made pretty good progress, but am a bit stuck with this problem. Basically: 003d:trace:ole:TL_Marshal parameter 1 003d:trace:ole:TL_Marshalname : L"rgszNames" 003d:trace:ole:TL_Marshaltype : 26 003d:trace:ole:TL_Marshalflags : 01 003d:trace:ole:TL_MarshalTypedereferencing PTR 0x52edf7d0 => 0x54331260 003d:trace:ole:TL_MarshalTypedereferencing PTR 0x54331260 => 0x54331270 003d:trace:ole:TL_MarshalTypemarshaling byte 100 As you can see, it's marshalling IDispatch across apartment boundaries, but it gets this parameter wrong. The double dereference seems correct, but marshalling a byte is not - I think it should be a BSTR. rgszNames is a parameter of IDispatch::GetIDsFromNames, which is defined as an OLECHAR FAR* FAR* I can't figure out where the typeinfo data for IDispatch is coming from though, hence my stuckness. BTW, is this line really correct: TL_Unmarshal(pStm, pInfo, pDesc, PARAMFLAG_FIN, args, &argc, pChannel, &is_iid); shouldn't it be: TL_Unmarshal(pStm, pInfo, pDesc, PARAMFLAG_FIN, (DWORD*)&args[0], &argc, pChannel, &is_iid); as args[] is a local array declared on the stack which is then passed to the function to be invoked - otherwise how does TL_Unmarshal set the arguments? Well I tried changing it, and it made no obvious difference to my bug, but I thought I'd ask anyway. thanks -mike
Re: Typelib containment refcounting (2)
Mike Hearn <[EMAIL PROTECTED]> writes: > This is a slightly modified version of the original patch which releases > the typelib reference on every ITypeInfo::Release(), not just when being > destroyed. This is still not right, there are many other places that increment the ITypeInfo refcount that would need to be fixed too. And if you increment the typelib refcount everywhere it will create a refcount loop and nothing will ever get freed. This needs more thought... -- Alexandre Julliard [EMAIL PROTECTED]
Re: TypeLib containment refcounting
Mike... First of all, sorry but I don't have a linux box right now (bad HDD) so this is the best I can do. My code below will cause an AddRef only on the first ITypeInfo pointer created (for each ITypeInfo), and a release on the last ITypeInfo pointer released. For each typeinfo struct in the typelib, you will get one reference if a client has any references to that object. As soon as all client references to ITypeInfo's supported by the typeinfo in the library are released, the refcount of the library will go to zero. Of course, you should not addref the library (directly) in GetTypeInfoOfGuid or any other place that doesn't return a ITypeLib pointer. Here's some source (with changes inline) from the typelib.c in 5/8/2003 source on source.winehq.com 3795 /* ITypeInfo::AddRef 3796 */ 3797 static ULONG WINAPI ITypeInfo_fnAddRef( ITypeInfo2 *iface) 3798 { 3799 ICOM_THIS( ITypeInfoImpl, iface); 3800 here, I'd recommend inserting: if(This->ref == 0) ITypeLib2_AddRef((ITypeLib2 *)This->pTypeLib); 3801 ++(This->ref); 3802 3803 TRACE("(%p)->ref is %u\n",This, This->ref); 3804 return This->ref; 3805 } 3806 3807 /* ITypeInfo::Release 3808 */ 3809 static ULONG WINAPI ITypeInfo_fnRelease( ITypeInfo2 *iface) 3810 { 3811 ICOM_THIS( ITypeInfoImpl, iface); 3812 3813 --(This->ref); and here, I'd recommend inserting if(This->ref == 0) ITypeLib2_Release((ITypeLib2 *)This->pTypeLib); 3814 3815 TRACE("(%p)->(%u)\n",This, This->ref); 3816 3817 if (!This->ref) 3818 { 3819 FIXME("destroy child objects\n"); 3820 3821 TRACE("destroying ITypeInfo(%p)\n",This); 3822 if (This->Name) 3823 { 3824 SysFreeString(This->Name); 3825 This->Name = 0; 3826 } 3827 3828 if (This->DocString) 3829 { 3830 SysFreeString(This->DocString); 3831 This->DocString = 0; 3832 } 3833 3834 if (This->next) 3835 { 3836 ITypeInfo_Release((ITypeInfo*)This->next); 3837 } 3838 3839 HeapFree(GetProcessHeap(),0,This); 3840 return 0; 3841 } 3842 return This->ref; 3843 }
Re: TypeLib containment refcounting
> Yes, that's what I think Alexandre's point was. An app can use > GetTypeInfoOfGuid twice, the same typeinfo object would be returned in > both calls, you addref the typelib twice, the app can then release that > typeinfo twice, but with the result that the typeinfo is only destroyed > once, so the typelib would be only released once with your patch. Ah, I understand now. Thanks for explaining this to me. > So, either addref the typelib only if the app does not already have > references to the typeinfo, or easier, always release the typelib in the > typeinfo's release whether or not it's the last reference (I'm not sure > you'd ever get to the last reference anyway because the typelib > structure keeps internal references to typeinfo objects too). Yeah, I think moving the TypeLib release to the TypeInfo release method would be the best option here. I will try and resubmit the patch in the next couple of days. thanks -mike -- Mike Hearn <[EMAIL PROTECTED]> QinetiQ - Malvern Technology Center
Re: TypeLib containment refcounting
tir, 17.06.2003 kl. 14.55 skrev Mike Hearn: > > > Well, unless you can call GetTypeInfoOfGuid twice and get two pointers to > > > the same interface (maybe you can, i don't really know but that would seem > > > unusual), I don't understand why this is wrong. > > > > That is exactly what you can in the current typelib implementation. The > > pointers will be the same, too. Whether you can in the real MS > > implementation I'm not sure, but shouldn't matter, since the current > > Wine implementation is what you're dealing with. > > Even so, as far as the users of these interfaces are concerned, they > have to be properly reference counted which the app I'm dealing with, > which is MS native code appears to do. > > So, is the problem here that I would be leaking references? Yes, that's what I think Alexandre's point was. An app can use GetTypeInfoOfGuid twice, the same typeinfo object would be returned in both calls, you addref the typelib twice, the app can then release that typeinfo twice, but with the result that the typeinfo is only destroyed once, so the typelib would be only released once with your patch. So, either addref the typelib only if the app does not already have references to the typeinfo, or easier, always release the typelib in the typeinfo's release whether or not it's the last reference (I'm not sure you'd ever get to the last reference anyway because the typelib structure keeps internal references to typeinfo objects too).
Re: TypeLib containment refcounting
> > Well, unless you can call GetTypeInfoOfGuid twice and get two pointers to > > the same interface (maybe you can, i don't really know but that would seem > > unusual), I don't understand why this is wrong. > > That is exactly what you can in the current typelib implementation. The > pointers will be the same, too. Whether you can in the real MS > implementation I'm not sure, but shouldn't matter, since the current > Wine implementation is what you're dealing with. Even so, as far as the users of these interfaces are concerned, they have to be properly reference counted which the app I'm dealing with, which is MS native code appears to do. So, is the problem here that I would be leaking references? -- Mike Hearn <[EMAIL PROTECTED]> QinetiQ - Malvern Technology Center
Re: TypeLib containment refcounting
man, 16.06.2003 kl. 21.28 skrev Mike Hearn: > > This doesn't look right, you increment the refcount on every GetTypeInfo > > but only decrement when the typeinfo object is destroyed. > > Well, unless you can call GetTypeInfoOfGuid twice and get two pointers to > the same interface (maybe you can, i don't really know but that would seem > unusual), I don't understand why this is wrong. That is exactly what you can in the current typelib implementation. The pointers will be the same, too. Whether you can in the real MS implementation I'm not sure, but shouldn't matter, since the current Wine implementation is what you're dealing with.
Re: TypeLib containment refcounting
> Another way is the addref the type library when the ITypeInfo wherever a > ITypeInfo interface is initially created (like in GetTypeInfoOfGuid) but I > think this is more dangerous, since then you have to have some way for this > refcount to go away, and ITypeInfo_Release() doesn't know who created it, > just that it was created. If you are careful enough to always put an addref > on the library when you create a new ITypeInfo pointer (GetTypeInfoOfGuid, > GetTypeInfo, IDispatch::GetTypeInfo, etc.) but this seems like a unwieldy > task to guarantee that the code is right in all of these places. It's much > easier to make the ITypeInfo_AddRef() and ITypeInfo_Release() call > ITypeLib_AddRef() and ITypeLib_Release() on the containing ITypeLib > implementation when appropriate. Yes, this is true. I hadn't considered that, it would be better to put the TypeLib addref in the TypeInfo addref. This feels a bit odd though, it would lead to the TypeLib having a reference count that was "too high" (though I suppose it doesn't really matter) if the receiver did their own AddRef on the TypeInfo, as the refs would propogate up. Still, the actual value of the refcount doesn't matter. > As far as C++ stuff vs. C stuff, in essence the pointer to the data > structure (including the VMT) is simply a C++ style class without the C++, > and with some special requirements... You can just replace my p->method() > with OBJECT_method(p) if you like to make it look like what I've seen from > wine. I'm still pretty new to the wine source, so it may not be like this > everywhere, I just don't have the time to look. True, what I meant was that there is no object constructor, unless you count the code where the struct is allocated and appended to the end of the linked list. thanks -mike -- Mike Hearn <[EMAIL PROTECTED]> QinetiQ - Malvern Technology Center
Re: TypeLib containment refcounting
- Original Message - From: "Mike Hearn" <[EMAIL PROTECTED]> To: <[EMAIL PROTECTED]> Sent: Monday, June 16, 2003 3:16 PM Subject: Re: TypeLib containment refcounting > On Mon, 16 Jun 2003 14:43:25 -0500, Sir Kelly Leahy scribed thus: > > I would expect you to addref the typelib when the ITypeInfo pointer is > > created, and then when it is destroyed to release it. > > I believe that's what I'm doing. Maybe not. > > I think what is actually going on, is that this method returns a pointer > to an internal interface that's always present. There is no construction > as such. From MSDN: > > HRESULT GetTypeInfoOfGuid( > REFGUID guid, > ITypeInfo FAR* FAR* ppTinfo > ); > Retrieves the type description that corresponds to the specified GUID. > > > This confuses me. Does the caller need to release the interface? The page > doesn't mention refcounting at all. > > "Retrieves" is somewhat fuzzy. The fact that it's a pointer to a pointer > suggests that maybe you get a pointer to an internal interface and you > shouldn't do any refcounting on it. > The caller should release the object when they're done with it. I guess what I was saying is that there's a few ways this can be done. One way is for ITypeInfo_AddRef() to addref the type library object that contains it (i.e. add one to the refcount on the ITypeLib struct in C terms). It can either do this every time (in which case, ITypeInfo_Release() should release the type library object each time) or it can do it only when the refcount is zero on entry to ITypeInfo_AddRef() (in which case ITypeInfo_Release() should only Release() the type library object when the result of decrementing the refcount is zero in ITypeInfo_Release()). Another way is the addref the type library when the ITypeInfo wherever a ITypeInfo interface is initially created (like in GetTypeInfoOfGuid) but I think this is more dangerous, since then you have to have some way for this refcount to go away, and ITypeInfo_Release() doesn't know who created it, just that it was created. If you are careful enough to always put an addref on the library when you create a new ITypeInfo pointer (GetTypeInfoOfGuid, GetTypeInfo, IDispatch::GetTypeInfo, etc.) but this seems like a unwieldy task to guarantee that the code is right in all of these places. It's much easier to make the ITypeInfo_AddRef() and ITypeInfo_Release() call ITypeLib_AddRef() and ITypeLib_Release() on the containing ITypeLib implementation when appropriate. > > In the object that implements the ITypeInfo reference, you should have a > > pTypeLib->Release() in the destructor and a pTypeLib->AddRef() somewhere in > > the code that creates the ITypeInfo implementing object. > > Well that's C++ stuff. Remember that all this is implemented in pure C. > The relevant code searches a linked list for a ITypeInfoImpl struct, which > stores all the data and the COM Vtable. It then casts it to an ITypeInfo > and incs the refcount: > > *ppTInfo = (ITypeInfo*)pTypeInfo; > ITypeInfo_AddRef(*ppTInfo); > > So, what you actually get out of this method is a pointer to an internal > data structure rather than a constructed object > Sorry... I didn't have the source handy to look at (RMAing my linux box HDD right now!) As far as C++ stuff vs. C stuff, in essence the pointer to the data structure (including the VMT) is simply a C++ style class without the C++, and with some special requirements... You can just replace my p->method() with OBJECT_method(p) if you like to make it look like what I've seen from wine. I'm still pretty new to the wine source, so it may not be like this everywhere, I just don't have the time to look. Kelly
Re: TypeLib containment refcounting
On Mon, 16 Jun 2003 14:43:25 -0500, Sir Kelly Leahy scribed thus: > I would expect you to addref the typelib when the ITypeInfo pointer is > created, and then when it is destroyed to release it. I believe that's what I'm doing. Maybe not. I think what is actually going on, is that this method returns a pointer to an internal interface that's always present. There is no construction as such. From MSDN: HRESULT GetTypeInfoOfGuid( REFGUID guid, ITypeInfo FAR* FAR* ppTinfo ); Retrieves the type description that corresponds to the specified GUID. This confuses me. Does the caller need to release the interface? The page doesn't mention refcounting at all. "Retrieves" is somewhat fuzzy. The fact that it's a pointer to a pointer suggests that maybe you get a pointer to an internal interface and you shouldn't do any refcounting on it. > In the object that implements the ITypeInfo reference, you should have a > pTypeLib->Release() in the destructor and a pTypeLib->AddRef() somewhere in > the code that creates the ITypeInfo implementing object. Well that's C++ stuff. Remember that all this is implemented in pure C. The relevant code searches a linked list for a ITypeInfoImpl struct, which stores all the data and the COM Vtable. It then casts it to an ITypeInfo and incs the refcount: *ppTInfo = (ITypeInfo*)pTypeInfo; ITypeInfo_AddRef(*ppTInfo); So, what you actually get out of this method is a pointer to an internal data structure rather than a constructed object
Re: TypeLib containment refcounting
I would expect you to addref the typelib when the ITypeInfo pointer is created, and then when it is destroyed to release it. In the object that implements the ITypeInfo reference, you should have a pTypeLib->Release() in the destructor and a pTypeLib->AddRef() somewhere in the code that creates the ITypeInfo implementing object. If, for instance, the ITypeLib pointer is passed to the constructor of this object, then the constructor should look something like... m_pTypeLib = pTypeLib; pTypeLib->AddRef(); ... and the destructor should look like m_pTypeLib->Release(); ... Kelly - Original Message - From: "Mike Hearn" <[EMAIL PROTECTED]> To: <[EMAIL PROTECTED]> Sent: Monday, June 16, 2003 2:28 PM Subject: Re: TypeLib containment refcounting > > This doesn't look right, you increment the refcount on every GetTypeInfo > > but only decrement when the typeinfo object is destroyed. > > Well, unless you can call GetTypeInfoOfGuid twice and get two pointers to > the same interface (maybe you can, i don't really know but that would seem > unusual), I don't understand why this is wrong. When somebody calls > GetTypeInfoOfGuid they get an interface, so at some point they should > release it, which will in turn release the reference to the typelib. > Without this patch, an app did a GetTypeInfo, released the typelib object > and then tried to get the container from ITypeInfo which obviously crashed > when it tried to do an AddRef on the destroyed object. > > Otherwise I don't really understand how this should work, unless you > manually add a reference for every type info object when the typelib is > first constructed, then manually release also for every object. That would > seem a bit odd though, to me. > > Could you or somebody else explain how this should work? > > thanks -mike > >
Re: TypeLib containment refcounting
> This doesn't look right, you increment the refcount on every GetTypeInfo > but only decrement when the typeinfo object is destroyed. Well, unless you can call GetTypeInfoOfGuid twice and get two pointers to the same interface (maybe you can, i don't really know but that would seem unusual), I don't understand why this is wrong. When somebody calls GetTypeInfoOfGuid they get an interface, so at some point they should release it, which will in turn release the reference to the typelib. Without this patch, an app did a GetTypeInfo, released the typelib object and then tried to get the container from ITypeInfo which obviously crashed when it tried to do an AddRef on the destroyed object. Otherwise I don't really understand how this should work, unless you manually add a reference for every type info object when the typelib is first constructed, then manually release also for every object. That would seem a bit odd though, to me. Could you or somebody else explain how this should work? thanks -mike
Re: TypeLib containment refcounting
Mike Hearn <[EMAIL PROTECTED]> writes: > ChangeLog: > When providing a TypeInfo object, inc/dec the refcount so that programs > don't call GetContainingTypeLib and crash as the container has been > destroyed. This doesn't look right, you increment the refcount on every GetTypeInfo but only decrement when the typeinfo object is destroyed. -- Alexandre Julliard [EMAIL PROTECTED]
Re: typelib patch 2
On Wed, Oct 03, 2001 at 04:35:14PM +0200, Ove Kaaven wrote: > As far as I can tell, the DISPATCH_METHOD flag shouldn't be masked out > in here... (Marcus, was there a reason for this?) No, I was just confused by the bad enum and lack of documentation mostly ;) Ciao, Marcus
Re: typelib patch 2
On Wed, 3 Oct 2001, Uwe Bonnes wrote: > > "Ove" == Ove Kaaven <[EMAIL PROTECTED]> writes: > ... > > Ove> With this patch, InstallShield6's user interface comes up. (It > Ove> seems to start to fail in some shell stuff, though) > With or without the installshield 6 patch on wine-devel? With it, of course.
Re: typelib patch 2
> "Ove" == Ove Kaaven <[EMAIL PROTECTED]> writes: ... Ove> With this patch, InstallShield6's user interface comes up. (It Ove> seems to start to fail in some shell stuff, though) With or without the installshield 6 patch on wine-devel? Bye -- Uwe Bonnes[EMAIL PROTECTED] Institut fuer Kernphysik Schlossgartenstrasse 9 64289 Darmstadt - Tel. 06151 162516 Fax. 06151 164321 --
Re: typelib
Rein Klazes wrote: > On Thu, 6 Jul 2000 20:31:13 +0200, you wrote: > > > Sounds good let's impement like this. Should I go for if? > > If I understand correctly, Francois Jacques has been assigned to fix > this and others issues with regard to ole automation. > True. But if Juergen feels like implementing it, I don't see any problem! This way I could concentrate my efforts on a few other issues. Cheers, Francois
Re: typelib
Juergen Schmied wrote: > > > > What I would suggest is a merge between your new implementation and Rein > > > > Klases' original code. The code could go like this... > > > I removed this code because when starting hh.exe it finds something what > > > is not a typelib and crashes. We would need a more safe way here... > > > > See also the documentation for load typelib: > > > > - first it is tried if the file is a stand-alone typelib > > - if that fails, then if it is a executable load the typelib from > > resource > > - if that fails parse the file into a moniker. > Sounds good let's impement like this. Should I go for if? > > MapFile > if(Signature = MSFT) > ReadTypelib(BaseAdress) > Unmap > > else if(Signature = MZ) > LoadLibrary() > GetResource() > ReadTypelib(ResourceAdress) > else > FIXME: case3 > > juergen > That sounds good to me!
Re: typelib
On Thu, 6 Jul 2000 20:31:13 +0200, you wrote: > Sounds good let's impement like this. Should I go for if? If I understand correctly, Francois Jacques has been assigned to fix this and others issues with regard to ole automation. Rein. -- Rein Klazes [EMAIL PROTECTED]
Re: typelib
> > > What I would suggest is a merge between your new implementation and Rein > > > Klases' original code. The code could go like this... > > I removed this code because when starting hh.exe it finds something what > > is not a typelib and crashes. We would need a more safe way here... > > See also the documentation for load typelib: > > - first it is tried if the file is a stand-alone typelib > - if that fails, then if it is a executable load the typelib from > resource > - if that fails parse the file into a moniker. Sounds good let's impement like this. Should I go for if? MapFile if(Signature = MSFT) ReadTypelib(BaseAdress) Unmap else if(Signature = MZ) LoadLibrary() GetResource() ReadTypelib(ResourceAdress) else FIXME: case3 juergen --- [EMAIL PROTECTED] ... from sunny Berlin
Re: typelib
On Thu, 6 Jul 2000 05:17:20 +0200, you wrote: > > Juergen, > > > > The new implementation can only load type libraries from .DLLs when the > > previous version could load the type libraries from any kind of file, more > > especially .TLBs > How are the typelibs stored in other files? Not as resources??? > What file contains such a typelib as example? Stand-alone typelibs are files (*.tlb, *.olb maybe more), produced by the midl compiler. The only example that I could find in a win98/system dir was msdatsrc.tlb, IIRC win95 has several more. Typelibs can be embedded as resources or as compound files. I have no example of the latter. > > > What I would suggest is a merge between your new implementation and Rein > > Klases' original code. The code could go like this... > I removed this code because when starting hh.exe it finds something what > is not a typelib and crashes. We would need a more safe way here... See also the documentation for load typelib: - first it is tried if the file is a stand-alone typelib - if that fails, then if it is a executable load the typelib from resource - if that fails parse the file into a moniker. Rein. -- Rein Klazes [EMAIL PROTECTED]
Re: typelib
> Juergen, > > The new implementation can only load type libraries from .DLLs when the > previous version could load the type libraries from any kind of file, more > especially .TLBs How are the typelibs stored in other files? Not as resources??? What file contains such a typelib as example? > What I would suggest is a merge between your new implementation and Rein > Klases' original code. The code could go like this... I removed this code because when starting hh.exe it finds something what is not a typelib and crashes. We would need a more safe way here... juergen --- [EMAIL PROTECTED] ... from sunny Berlin
Re: typelib
Juergen, The new implementation can only load type libraries from .DLLs when the previous version could load the type libraries from any kind of file, more especially .TLBs What I would suggest is a merge between your new implementation and Rein Klases' original code. The code could go like this... --> hModule = LoadLibrary... if (!hModule) { /* not a DLL, try to find tlbs anyway */ if ( /* try_tlb_hack_and_set_pLib() == FAILED */ ) { /* clean up (or goto cleanup) and exit */ } } else { /* it's a DLL! resource search and load ... eventually pLib get set */ } /* pLib is set and we can continue... */ <-- Cheers, Francois Juergen Schmied wrote: > - removed hack to find the right resource > - updated to use the ICOM macros > - cleaned up use typelib and typelib2 (was mixed up) > > --- > [EMAIL PROTECTED] > > ... from sunny Berlin > > > The following section of this message contains a file attachment > prepared for transmission using the Internet MIME message format. > If you are using Pegasus Mail, or any another MIME-compliant system, > you should be able to save it or view it from within your mailer. > If you cannot, please ask your system administrator for assistance. > > File information --- > File: Typelib.dif > Date: 24 Jun 2000, 9:48 > Size: 95844 bytes. > Type: Unknown > > > Name: Typelib.dif >Typelib.difType: unspecified type (Application/Octet-stream) > Encoding: BASE64
Re: More typelib fixes
Huw, > By the way can anybody point me to the spec of a typelib file format? There is no MS typelib file format specification out there :-( The thing being public about their file format is what's in it, and that's documented in MSDN, especially inside Books/Inside OLE (yes, the whole Inside OLE book is in MSDN!) Francois Jacques Macadamian Technologies Inc.
Re: More typelib fixes
At 05:49 PM 7/1/00 +0200, you wrote >If you have a typelib that is not decoded correctly I'd be interested >to have a look. I have the impression that typelib that are not included in a dll are not working. TLB_ReadTypeLib begins by a LoadLibraryEx call, but if the call fails there is no attempt to read the file directly. I don't think that the incredible Ms hack to specify the number of the resource after the name of a dll is supported, either. But that's not really about 'decoding' of course. Gerard
Re: More typelib fixes
On Sat, Jul 01, 2000 at 05:49:51PM +0200, Rein Klazes wrote: > On Sat, 1 Jul 2000 14:11:55 +0100, you wrote: > > > > > > By the way can anybody point me to the spec of a typelib file format? > > typelib.c (c) Rein Klazes ;) Hmmm, I thought as much. > If you have a typelib that is not decoded correctly I'd be interested > to have a look. It relates to the BSTR with length set to -1. I'm not sure my fix really handles this correctly. Examples include msword9.olb and vbe6.dll Huw. -- Dr. Huw D M Davies | Clarendon Laboratory [EMAIL PROTECTED] | Parks Road Tel: +44 1865 272390| Oxford OX1 3PU Fax: +44 1865 272400| UK
Re: More typelib fixes
On Sat, 1 Jul 2000 14:11:55 +0100, you wrote: > > By the way can anybody point me to the spec of a typelib file format? typelib.c (c) Rein Klazes ;) If you have a typelib that is not decoded correctly I'd be interested to have a look. Rein. -- Rein Klazes [EMAIL PROTECTED]
Re: COREL: typelib speedup
Alexandre Julliard wrote: > Maybe now is a good time to discuss the implementation issues: > I suggest that we put the full text of the license (since it is short > enough) at the top of every source file. I'd also like to hear > opinions about the copyright notices; the ones we have now are useful > in finding the person to annoy with questions about a given file, but > they are not strictly correct WRT the copyright law, since they don't > list all copyright holders. Suggestions? currently, the names appearing in the files list (more or less) the major contributors to the file. However, some names listed in the files are from people who no longer contribute to the project, so it won't be usefull to "annoy" them so, there are two different concepts that need to be addressed: - copyright holders on the file. normally, those should be the list of people who did make a patch to that file. but that's a rather fuzzy definition: what about if a file is split (at one day or the other) in two files => will a contributor to first file have copyrights on the two new files (even if his contribution only applied to what became, let's say, the first file) ? - maintainer, contact person for the function => given the current number of wine's contributor it might be difficult to "assign" maintainers to a file (or even a module or DLL) so, we could either : - remove all of the names from the files (there's no real need from a project development point of view) - or list the "major contributors" (to tackle the developer's susceptibility issue), but it'll be hard to define clearly what a "major contributor" is (even a one line fix can be a major contribution) A+ -- --- Eric Pouech (http://perso.wanadoo.fr/eric.pouech/) "The future will be better tomorrow", Vice President Dan Quayle
Re: COREL: typelib speedup
Ove Kaaven <[EMAIL PROTECTED]> writes: > > Changelog: > > Alexandre Julliard <[EMAIL PROTECTED]> (for Corel) > > I'm sure there must be other people than me wondering... which Alexandre > is this who needs to have his patch submitted to Wine by someone else? It is the Alexandre-working-on-behalf-of-Corel, not the Alexandre-hacking-Wine-on-his-own-time; they are quite different animals. The work I do for Corel is owned by them, and they can do what they want with it; it may or may not end up in their CVS tree, and as long as it isn't released to the public I don't have any right to put it in the WineHQ tree. Once it shows up in the Corel tree I could of course merge it into WineHQ myself, but if other people do the work for me I'm not going to complain... Even though Corel are very cool about this, and of course intend to release all the Wine code back to the community anyway, I think it is important to make a clear distinction, to ensure that I don't take credit for changes paid for by Corel (or that they take credit for changes I do on my own...) This is also why I have adopted the convention of putting my CodeWeavers email in the log entry of all the patches I did on behalf of Corel. > (And how is the license change going anyway, Alexandre?) Pretty good, I have received an OK from about 75 people, and more are still arriving. Since I haven't received any opposition so far I think it is safe to say that we will be able to do the change pretty soon. Maybe now is a good time to discuss the implementation issues: I suggest that we put the full text of the license (since it is short enough) at the top of every source file. I'd also like to hear opinions about the copyright notices; the ones we have now are useful in finding the person to annoy with questions about a given file, but they are not strictly correct WRT the copyright law, since they don't list all copyright holders. Suggestions? -- Alexandre Julliard [EMAIL PROTECTED]
Re: COREL: typelib speedup
On Fri, 18 Feb 2000, Marcus Meissner wrote: > Changelog: > Alexandre Julliard <[EMAIL PROTECTED]> (for Corel) I'm sure there must be other people than me wondering... which Alexandre is this who needs to have his patch submitted to Wine by someone else? (And how is the license change going anyway, Alexandre?)