Mike Hearn wrote:
This patch makes InstallShield work again, and should let you get further Bill.
- Ensure all intermediate threads join the original apartment
- Make the listener thread fully apartment scoped (one per apt not per
process)
- Improve "already have mid" error message
- Rename _StubReaderThread to stub_dispatch_thread
- Improve tracing
Sorry Mike, but we want to *remove* the current hacks for the RPC-runtime emulating named pipe code, not adding more. The correct approach for the crash in _LocalServerThread is to use table-strong marshaling once and then copy the contents of the stream to the pipe for each iteration in the loop, instead of marshaling each time. The correct approach for the crash in _StubReaderThread is to make it execute in the correct apartment - either by sending a window message to the STA window or by joining the MTA. I believe you had a patch for this in July or August when we last worked on this.
Rob
On Mon, 2004-12-20 at 17:56 +0000, Robert Shearman wrote:
Sorry Mike, but we want to *remove* the current hacks for the RPC-runtime emulating named pipe code, not adding more.
Yes, of course, but we also want to keep the code in CVS in a roughly working situation. As it is, the code is guaranteed to crash with all InstallShields (or indeed any program that uses out of process COM). If Alexandre was to do a new tarball release tomorrow that's a lot of users who are now inconvenienced.
Currently table marshalling isn't implemented, nor is thread affinity. They're not 5-line patches either. They are on the todo list though and when they're done we can redo the threading to work more like native and remove the hacks.
IIRC the patch I wrote for thread affinity at the start of the summer was never submitted precisely because it used a hack to ensure the thread could find its wine_pipe again (a TLS slot/stack thing). I think for thread affinity to work correctly we need to at least rework how pipes and threads interact, maybe even switch it on top of the RPC runtime transports entirely so re-entrancy works correctly.
The patch really isn't as big as it looks, there are some formatting changes in there and some renaming+commenting.
thanks -mike
Mike Hearn wrote:
On Mon, 2004-12-20 at 17:56 +0000, Robert Shearman wrote:
Sorry Mike, but we want to *remove* the current hacks for the RPC-runtime emulating named pipe code, not adding more.
Yes, of course, but we also want to keep the code in CVS in a roughly working situation. As it is, the code is guaranteed to crash with all InstallShields (or indeed any program that uses out of process COM). If Alexandre was to do a new tarball release tomorrow that's a lot of users who are now inconvenienced.
Currently table marshalling isn't implemented, nor is thread affinity. They're not 5-line patches either. They are on the todo list though and when they're done we can redo the threading to work more like native and remove the hacks.
IIRC the patch I wrote for thread affinity at the start of the summer was never submitted precisely because it used a hack to ensure the thread could find its wine_pipe again (a TLS slot/stack thing). I think for thread affinity to work correctly we need to at least rework how pipes and threads interact, maybe even switch it on top of the RPC runtime transports entirely so re-entrancy works correctly.
The patch really isn't as big as it looks, there are some formatting changes in there and some renaming+commenting.
thanks -mike
I just want to say that I tried out this patch. It does not fix the problem for InstallShield 6. I have a bug report in bugzilla if you are interested. Right now AFAIKT InstallSheild is seriously broken (for quite some time) and I am unable to use Native ole to workaround the problem. If there is anyway I can help this process I please tell me.
--
Tony Lambregts.
tony_lambregts@telusplanet.net wrote:
I just want to say that I tried out this patch. It does not fix the problem for InstallShield 6. I have a bug report in bugzilla if you are interested. Right now AFAIKT InstallSheild is seriously broken (for quite some time) and I am unable to use Native ole to workaround the problem.
Why does native ole not work?
If there is anyway I can help this process I please tell me.
If you don't want to assist us in the programming work, then you can help us by testing things that should work (I'll include a comment in my message to wine-patches if I think something should start working with the patch). Each program's InstallShield seems to be unique and they usually break in unique ways, so any help with testing different installers would be greatly appreciated.
Rob
Robert Shearman wrote:
tony_lambregts@telusplanet.net wrote:
I just want to say that I tried out this patch. It does not fix the problem for InstallShield 6. I have a bug report in bugzilla if you are interested. Right now AFAIKT InstallSheild is seriously broken (for quite some time) and I am unable to use Native ole to workaround the problem.
Why does native ole not work?
Because I did not have stdole32.tlb since I usually run wine with no windows files. ;^) When I applied Mikes patch and imported a stdole32.tlb file from windows the program installs.
Not having our own version of stdole32.tlb means that the at least InstallShield 6 will not work "out of the box".
We have had a couple of proposals for implementing our own version of stdole32.tlb. Huw Davies' is the most recent, it was submitted to wine patches by Vincent Béron. http://www.winehq.org/hypermail/wine-patches/2004/12/0053.html. and the most recent discussion thread is here. http://www.winehq.org/hypermail/wine-devel/2004/12/0164.html . The upshot of which was that Alexandre said that it should be installed at compile time.
Anyways I was wondering I anyone could give me an update.
If there is anyway I can help this process I please tell me.
If you don't want to assist us in the programming work, then you can help us by testing things that should work (I'll include a comment in my message to wine-patches if I think something should start working with the patch). Each program's InstallShield seems to be unique and they usually break in unique ways, so any help with testing different installers would be greatly appreciated.
Rob
I am currently building wine with Vincent's and Mike's patches applied to see if it will install without native stdole32.tlb. I will let you know what the results are.
On Wed, 2004-12-22 at 14:18 -0700, tony_lambregts@telusplanet.net wrote:
Anyways I was wondering I anyone could give me an update
Huw is working on adding TLB output support to widl, so hopefully we should have a stdole32.tlb out of the box within a month or so (maybe earlier).
The rest of the work involves simply improving our DCOM code to make InstallShield work correctly. Here are the issues I know about already:
- stdole32.tlb needed, see above - painting problems (ie it doesn't draw right): needs thread affinity, which in turn needs correct RPC re-entrancy - iTunes installer hangs at the end, refcount leak triggered I think by InstallShield throwing across non-exception safe code when it hits an MSI stub - InstallShield 8 (9?) doesn't start up ...
thanks -mike
On Mon, 2004-12-20 at 16:39 -0700, tony_lambregts@telusplanet.net wrote:
I just want to say that I tried out this patch. It does not fix the problem for InstallShield 6.
I'm afraid that's a bit vague, there are still lots of problems with InstallShield 6 we know about. The patch I posted for me makes the FashionCam01 installer start up and run to completion again when given a native stdole32.tlb file, ie it's the same state it was after Marcus finished his initial work at least for the InstallShield I tried.
All the InstallShields I've seen so far go through the same basic steps DCOM-wise, though as Rob notes they are also all unique.
Native OLE should still work - is this the REG_EXPAND_SZ issue Bill mentioned? What exactly is this issue?
On December 21, 2004 04:25 am, Mike Hearn wrote: SNIP
Native OLE should still work - is this the REG_EXPAND_SZ issue Bill mentioned? What exactly is this issue?
(It was mentioned early on in that IRC hecking discussion a couple of Sundays ago, although I met it a few days earlier)
Due to some change after the September version, native msxml registers itself using REG_EXPAND_SZ entries in the registry, which is what it seems to do on a Windows 2000 computer. However the native ole32 that I use (which comes from I can't remember where) does not expand the string and so the msxml stuff stops working. I suggest as a working hypothesis that the Win95/98 ole32 does not support the REG_EXPAND_SZ. (Does the Windows 98 Registry code actually allow you to add a REG_EXPAND_SZ to the registry or doe it maybe automatically do the expansion on the way into the registry?)
On Tue, 2004-12-21 at 11:41 -0800, Bill Medland wrote:
I suggest as a working hypothesis that the Win95/98 ole32 does not support the REG_EXPAND_SZ. (Does the Windows 98 Registry code actually allow you to add a REG_EXPAND_SZ to the registry or doe it maybe automatically do the expansion on the way into the registry?)
I think DCOM98 is just buggy. That sort of thing works on Windows XP. Unfortunately XP DCOM doesn't work on Wine, so you have a few options:
- Help us improve builtin DCOM (yay!) - Hack the registry and manually expand the strings so MSXML is happy
If you're able to commit time to the first one then perhaps Rob and I should draw up a more detailed roadmap. Quite a few of the tasks can be done in parallel.
Of course, first we have to resolve the matter of the patch I sent earlier ...
On December 21, 2004 11:59 am, Mike Hearn wrote:
On Tue, 2004-12-21 at 11:41 -0800, Bill Medland wrote:
SNIP
- Help us improve builtin DCOM (yay!)
- Hack the registry and manually expand the strings so MSXML is happy
If you're able to commit time to the first one then perhaps Rob and I should draw up a more detailed roadmap. Quite a few of the tasks can be done in parallel.
I'm certainly interested but I think that I am not going to have the time. (Presumably it will take quite a while to get up to speed).
Of course, first we have to resolve the matter of the patch I sent earlier ...
On Tue, 2004-12-21 at 12:46 -0800, Bill Medland wrote:
I'm certainly interested but I think that I am not going to have the time. (Presumably it will take quite a while to get up to speed).
I don't know. Some tasks are not that hard, for instance making apartments into thread-safe COM objects. Others are more tricky. I'm happy to answer any questions you have, if you decide to go ahead.
thanks -mike
Mike Hearn wrote:
On Mon, 2004-12-20 at 17:56 +0000, Robert Shearman wrote:
Sorry Mike, but we want to *remove* the current hacks for the RPC-runtime emulating named pipe code, not adding more.
Yes, of course, but we also want to keep the code in CVS in a roughly working situation. As it is, the code is guaranteed to crash with all InstallShields (or indeed any program that uses out of process COM). If Alexandre was to do a new tarball release tomorrow that's a lot of users who are now inconvenienced.
And those same users would have been inconvenienced with the 20041201 release. It wouldn't crash, but it's likely the program still wouldn't work.
Currently table marshalling isn't implemented, nor is thread affinity. They're not 5-line patches either. They are on the todo list though and when they're done we can redo the threading to work more like native and remove the hacks.
I'll see what I can come up with this week. I know you have some work on the stub manager outstanding and I think it would be fairly easy to build on that work to implement the table marshaling. Thread affinity (dispatching calls to the right apartment) should also be quite easy to do, although not a 5-line patch, of course.
IIRC the patch I wrote for thread affinity at the start of the summer was never submitted precisely because it used a hack to ensure the thread could find its wine_pipe again (a TLS slot/stack thing). I think for thread affinity to work correctly we need to at least rework how pipes and threads interact, maybe even switch it on top of the RPC runtime transports entirely so re-entrancy works correctly.
I think we discussed about using a field in the RPCMESSAGE reserved for the RPC runtime (which is what we are emulating here). I don't think any other reworking is required.
The patch really isn't as big as it looks, there are some formatting changes in there and some renaming+commenting.
The main changes I object to are adding more fields to structures that I was previously pruning of hacks.
Rob
On Tue, 2004-12-21 at 10:55 +0000, Robert Shearman wrote:
And those same users would have been inconvenienced with the 20041201 release. It wouldn't crash, but it's likely the program still wouldn't work.
Well, I think an engineering goal for us should be that WineHQ CVS is at all times:
a) Internally consistent b) Working (even if it's not perfect)
This is good for several reasons. Obviously it helps users, but it also helps us: more people can help out, whereas if the code is missing pieces only we know about anybody who is interested in coding alongside us will fall at the first hurdle (I was talking to somebody who seemed to know DCOM pretty well in IRC last night, no idea if he'll turn up and help but you never know).
It helps in another way: it's easier to spot regressions if we have a baseline of "it works". That way when it stops working, we can figure out exactly which patch did it quickly. If the code simply crashes within a few yards of the first CoMarshalInterface call because we didn't put in the 6 lines of code necessary to keep it working during the transition, we have no idea if the code that comes later really works.
If we're not going to try and keep CVS working then we shouldn't be submitting patches there, especially if native DCOM is busted. We should be working in a private arch or subversion/svk branch where it doesn't matter if we go a year with no InstallShield support.
I'll see what I can come up with this week. I know you have some work on the stub manager outstanding and I think it would be fairly easy to build on that work to implement the table marshaling.
OK, you have the latest patch in your inbox. It's different from the previous but not much. If you think it's OK I'll go ahead and submit it (and I'll check it still works with InstallShield too).
Once that patch is in you're right, table marshalling should be easy. At that point we can start to remove some of the extra threads (there are 3 helper threads used currently, right?). The ones that are left will still need to join the marshalling apartment though, so this patch will still be necessary.
I think we discussed about using a field in the RPCMESSAGE reserved for the RPC runtime (which is what we are emulating here). I don't think any other reworking is required.
I couldn't see at the time how that'd work: the problem is we need to keep track of information across arbitrary application stack frames, ie
IRpcStubBuffer::Invoke -> InstallShield code -> proxy call -> pipe?!?
We can't transfer information across the InstallShield code without using a TLS slot. In native DCOM this doesn't matter as the proxy doesn't need to use the same pipe that the RPC came in from, it'll be using a different connection. In our current code, that won't work as the original caller can only re-enter from RPCs incoming on the same pipe it originally called out on. Native RPC doesn't use a thread per stub manager, and each outgoing call that could re-enter spawns another thread that dispatches incoming RPCs ad-infinitum.
Ugh, that's clear as mud I know. The issue is that our threading is wrong: we need more instances of the threads so re-entrancy works correctly. Before we change anything in this area then we need:
a) the unit tests you wrote to be in CVS b) more tests for out of process COM and out of process call re-entrancy
Once we can check the state of the code with more than just the random InstallShields we have, then I think it'll be OK to rework the threading/pipe management so thread-affinity works correctly.
The main changes I object to are adding more fields to structures that I was previously pruning of hacks.
While we have any helper threads that don't correspond exactly to what native DCOM does they'll need to enter the original apartment, I don't see any way around that. Simply not making them join will mean the current code is not only hacky but also wrong, which goes back to the first point. During the transition to real DCOM some hacks will be needed to support the original infrastructure, it's unavoidable. They can and will be removed as the rest of the code allows it.
We're both keen to get correct code as soon as possible, but this is a long term project and we want to keep disruption to a minimum.
thanks -mike
-
Bill Medland -
Mike Hearn -
Robert Shearman -
tony_lambregts＠telusplanet.net