DllMain & CoInitialize/CoUninitialize

[gabest]

msdn writes: "Because there is no way to control the order in which in-process servers are loaded or unloaded, it is not safe to call CoInitialize, CoInitializeEx, or CoUninitialize from the DllMain function."

I guess this isn't exactly the problem with media player classic when loading an avs file and deadlocking upon exit, but instead that I initialize COM as multi-threaded and avisynth tries to init it as single-threaded (when I also init mpc's thread as single-threaded, there is no deadlock). I can't debug avisynth now (no vc6 installed...) but I'm sure that CoInitialize(NULL) fails at DLL_PROCESS_ATTACH with the RPC_E_CHANGED_MODE return code, and since CoUninitialize() executes without knowing this under DLL_PROCESS_DETACH something very nasty happens there.

It would be better to move these calls away from DllMain or remove them completly (only apps should really init COM), but at least to check the return code of CoInitialize(NULL) and not call CoUninitialize() when it failed.

[sh0dan]

OK - I see the code, but I have absolutely no idea what it does, how it works. It's very old code - probably only Ben RG knows why it is like this.

I am however interested in this, since this could, perhaps, help the (sometimes random) problems with other players than vdub.

However - I'm not touching this myself, as I'm much more likely to break stuff than fix it.

You can however download the lastest main.cpp (http://cvs.sourceforge.net/cgi-bin/viewcvs.cgi/*checkout*/avisynth2/avisynth/main.cpp?rev=1.2&only_with_tag=MAIN) from CVS, and attach your suggested changes here.

[gabest]

1st variation: (making it a bit safer)static HRESULT hrfromcoinit = E_FAIL;
BOOL APIENTRY DllMain(HANDLE hModule, ULONG ulReason, LPVOID lpReserved) {

switch(ulReason) {
case DLL_PROCESS_ATTACH:
hrfromcoinit = CoInitialize(NULL);
_RPT0(0,"Process attach\n");
break;

case DLL_PROCESS_DETACH:
if(SUCCEEDED(hrfromcoinit)) CoUninitialize();
_RPT0(0,"Process detach\n");
break;
}

return TRUE;
}2st variation: (application loading avisynth must call coinit before)BOOL APIENTRY DllMain(HANDLE hModule, ULONG ulReason, LPVOID lpReserved) {

switch(ulReason) {
case DLL_PROCESS_ATTACH:
_RPT0(0,"Process attach\n");
break;

case DLL_PROCESS_DETACH:
_RPT0(0,"Process detach\n");
break;
}

return TRUE;
}

[sh0dan]

ok - I put in the top one in the lastest test binary.

Could you explain - what this means in real life, and what the result might be? I have no idea what this is about. Would the latter be safer, or should something else be rewritten?

[gabest]

Originally posted by sh0dan
ok - I put in the top one in the lastest test binary.Thanks, now it no longer deadlocks when unloading it. That couninit was the problemCould you explain - what this means in real life, and what the result might be?I think only the real experts of COM understand this, after reading many threads about it, I still can't understand it fully, but I'm sure there is a reason. (http://groups.google.com/groups?q=DllMain+CoInitialize&ie=UTF-8&oe=UTF-8&hl=en)

edit:
Just found something: "You shouldn't try to call CoInitialize or OleInitialize from a DLL, especially from DllMain, as the calls require the function to be reentrant, and DllMain is not."
I also read yesterday that coinit may load several other dlls too, and if it tries to load/unload avisynth again (which is locked in DllMain) that can deadlock.
I have no idea what this is about. Would the latter be safer, or should something else be rewritten? Since the first one is working fine, I'd say leave it as it is. Even this little mod fixed one bug and not introduced another, which is a good result after all :)

[IanB]

Dusting off old problems...[ 1624587 ] Deadlock after trying to unload dll (http://sourceforge.net/tracker/index.php?func=detail&aid=1624587&group_id=57023&atid=482673)

All programs that use avisynth keep deadlocking when they terminate. I found a doom9 forum post from gabest that seems to describe the problem: [This]

I think that it has to do with the DllMain function and CoUninitialize. I couldn't test it on my own. The problem occurred only with my intel core2duo T2300. The same build was running without problems with my old Athlon XP.

On top of that Virtual Dub, MPC etc. use 100% cpu power in idle cycle.

I wonder whether this could be fixed.
ThanksI was thinking along the lines of deferring the CoInit until the first substantial call to Avisynth (one that needs some level of resourses). And watching the global use count for zero to do the CoUnInit and/or doing it in the DllMain/DLL_PROCESS_DETACH if/when UseCount==0

Does anyone have any further thoughts on the matter.

[Haali]

Why not call CoInitialize() when script environment is constructed and CoUninitialize() during destruction? It's safe to call them multiple times. Also it's not clear why these calls are needed at all. Normally the calling application is responsible for properly initializing COM.

[IanB]

.... Also it's not clear why these calls are needed at all. Normally the calling application is responsible for properly initializing COM.And this is really where I was hoping to get some insight. Like Sh0dan, 4 years ago, I am reluctant to fiddle with what I don't yet understand.

Taking them out doesn't seem to harm anything in the 5 minute test I made, but someone, sometime, put that code there, Why?

One guess I thought of maybe it was something to do with DirectShowSource() when it was part of the core.

Another guess is it might be a kindness to some COM unaware app that directly pokes the AVI world or uses AVS directly.

Thoughts please!

[Richard Berg]

Another guess is it might be a kindness to some COM unaware app that directly pokes the AVI world or uses AVS directly.
That's my guess. Without it, an app driving Avisynth via the C++ interface would die when it tried to use a COM function like AviSource. CreateScriptEnvironment() sounds like a good place to put it.

/not a COM expert either

[Ebobtron]

gabest's first example is the way it needs to be done.

Current doc's say "for every CoIn there should be a CoUnIn", should read "for every successful CoIn there must be a CoUnIn" if COM is up you can not bring up a second instance, CoIn fails.

My recent experience suggests it is very simple, maybe.
if(CoInitialize() == S_FALSE)
{
"The COM library is already initialized on this thread."
(It didn't initialize itself so what ever did initialize the
interface, we must assume, will still need it.)
}
If you need COM initialize it, then uninitialize it if you initialized it, otherwise leave it be.
hrMyCom = CoInitialize()

MyComCode{}

if(hrMYCom == S_OK)
CoUnInitialize();It is unclear to me now, however, if one should restore the COM if it gets uninitialized unintentionally by an error or mistake in associated code. This would require you to create some kind of "was com up before I did this" code.
I have a case where drag and drop cutting and pasting stops working in the Scintilla edit control if I allow the COM to be uninitialized. Happily reinitializing COM restores the drag and drop without restarting Scintilla.

I was so proud of myself, I fixed the faulty code, took out the bypass code for "the direct show viewer/player", and watched sadly as I released the last filtergraph pointer which disabled the drop and drag in Scintilla.

The fix is "OleInitialize()" following the direct show shutdown.Typically, the COM library is initialized on an apartment only once. Subsequent calls will succeed, as long as they do not attempt to change the concurrency model of the apartment, but will return S_FALSE. To close the COM library gracefully, each successful call to OleInitialize, including those that return S_FALSE, must be balanced by a corresponding call to OleUninitialize.

Now I have to learn about apartments, I thinks me needs a new landlord. :)
Rob

[squid_80]

Current doc's say "for every CoIn there should be a CoUnIn", should read "for every successful CoIn there must be a CoUnIn" if COM is up you can not bring up a second instance, CoIn fails.
No, that's not what the current docs say.
http://msdn2.microsoft.com/en-us/library/ms688715.aspx
A thread must call CoUninitialize once for each successful call it has made to CoInitialize or CoInitializeEx, including any call that returns S_FALSE.

[Ebobtron]

@squid_80
No, that's not what the current docs say.
http://msdn2.microsoft.com/en-us/library/ms688715.aspx
Oops, sorry you are correct, my very bad. I have not a clue what is going on.

I do have some evidence that the documentation is at least partly incorrrect based upon my limited observations.

"Only the CoUninitialize call corresponding to the CoInitialize or CoInitializeEx call that initialized the library can close it."

if I // at this point there is no com
createwindowex(null,"scintilla", ...)
coinitialize()
// returns S_FALSE which mean the com is up on this thread
// the ole drag and drop cut and paste works in the edit control
couninitialize() // ole drag and drap gone
coinitialize() // returns S_OK and ole drag and drop returns to the edit control. I know this is an assuption on my part but if Scintilla initializes the COM why can I uninitialize it from FilmCutter? Why can I return the COM with initialize? All which acts like a ole on and off switch to Scintilla. Nothing appears to be broken, memory and thread counts are stable.

:confused:

Based solely on the "Only the CoUninitialize call corresponding to ......" when I saw that COM was up why should I attemp to uninitialize it except that they tell me to. Also they tell us to uninitialize at program termination, would we even notice a problem. Maybe they should be more clear like "if you initialize, uninitialize immediately before program termination". Or maybe they could be more simplistic, "if you need a com interface use CoInitialize to make sure that it is useable then add CoUnInitialize to your exit code." Sounds to me like that is what they are saying, so why use the remark "Only the CoUninitialize call corresponding to the CoInitialize or CoInitializeEx call that initialized the library can close it" when it appears that it is not true.

I can be horribly narrow minded, but Microsoft's remarks here are kind of like a Catholic school girl in the fifties, yes, no, yes, no (not that there is anything wrong with that).

I really don't know either. :)

Rob

[Haali]

This init stuff stuff is rather simple, COM maintains a reference count, CoInitialize increments it, CoUninitialize decrements. When it goes from 0 to 1 COM is initialized, from 1 to 0 - shutdown.

If you want to use OLE D&D you need to use OleInitialize/OleUninitialize instead, they do some extra work to enable this functionality.

[IanB]

A thread must call CoUninitialize once for each successful call it has made to CoInitialize or CoInitializeEx, including any call that returns S_FALSE.And I think the answer to all this is The Thread that calls CoInitialize must be The Thread that calls CoUninitialize. Best guess, some dumb schmuck is using some thread local storage that any other thread of coarse can't get at. The way things are, there is no correlation between the thread that loads avisynth.dll and the thread that finally does the last close of avisynth.dll.

I think I am begining to like the idea about CreateScriptEnvironment.

[squid_80]

And I think the answer to all this is The Thread that calls CoInitialize must be The Thread that calls CoUninitialize.
Indeed, I appear to be running into this problem today with an app that calls the CreateScriptEnvironment function in avisynth directly and then invokes directshowsource. The problem only shows itself when a particular (multithreaded) directshow decoder is used. What I'm wondering is if a call to CoInitialize returns RPC_E_CHANGED_MODE, does there still need to be a matching CoUnitialize call?

[Ebobtron]

GentlemanSorry, I wish I had more to contribute than more problems. I have come only one step closer to my problem.

Scintilla the edit control internally uses OleInitialize()

When I call CoInitialize() before I get my first IGraphBuilder pointer, it returns "S_FALSE" because the documentation says OleInitialize() calls CoInitializeEx(NULL,COINIT_APARTMENTTHREADED).
Changing CoInitialize() to CoInitializeEx(NULL,COINIT_MULTITHREADED) returns RPC_E_CHANGED_MODE, this confirms that the documentation appears to be correct.

Just wishing to nail down that the concurrency model used for incoming calls to objects created by this thread are apartment-threaded.

Using pGraphBlder->RenderFile(wfile,NULL) with Wav, Mpg, vob files followed by pGraphBlder->Release() would appear to leave Scintilla's IDropSource and IDropTarget objects intact or at least they continue to work.

Using pGraphBlder->RenderFile(wfile,NULL) with AviSynth script files followed by pGraphBlder->Release() would appear to impair only the use of the IDropSource object ( drag and drop inside of Scintilla no longer works and dragging to another application also discontinues ), the IDropTarget object continues to work ( I can drop text from another application into Scintilla running in my container ).

Understanding that I am the weakest link here I spent some time writing to Scintilla's author to confirm that the trouble wasn't the way I used Scintilla or what hasn't been completely confirmed, the way I code in general. During this exchange I have learned the following things.

Following the creation of the Scintilla window(control), alternating calls to CoUninitialize and CoInitialize, result in a kind of "off / on" switch for Scintilla's drag and drop, which I guess should make some sense. A call to CoInitialize after releasing the IGraphBuilder interface returns "S_FALSE" and does not restore the drag and drop object. Guess this means something else is happening.

Using OleInitialize after releasing the IGraphBuilder interface returns "S_FALSE" and does, I repeat does, restore drag and drop to the Scintilla window. This was a surprise until I read the following from the PSDK (R2) documentation. Typically, the COM library is initialized on an apartment only once. Subsequent calls will succeed, as long as they do not attempt to change the concurrency model of the apartment, but will return S_FALSE.

Also I discovered that repeated calls to OleInitialize (let us use three of them) after Scintilla creation but before any call to render a file with the filter-graph manager allows the use of my direct show interface with AviSynth files fours times before drag and drop don't work no more.

And then there is:Attaching using "env = CreateScriptEnvironment" has not caused problems with the Scintilla editor's drag and drop functions.

Also the AVIFile library calls I use to save a script's audio as a wave, or get script output properties, never seem to cause any trouble.
Wish I could test it but I can not build AviSynth here and my current skill level would only get works or don't work.

I have maybe done something to help. I have created a parallel application so that I can test code for FilmCutter without ripping FilmCutter to shreds when there is a problem. The current application sports the scintilla editor and a simple implementation of the graph builder, it needs a little clean up but it might be of help. If interested I can put it together and send you a link.

My current solution is to call OleInitialize if(file == avs) before exposing the direct show interface. This seems to cause the least of ills.

I have no clue which is the best course to follow but it seems to me that if on any given thread if COM is, leave COM alone.

Now here is a rub, when I render an avisynth script my thread count goes up by six. One for the graph manager and one for each filter. As I remove each filter the thread count becomes one less until all filters in the graph are removed, but until I release the filter-graph manager the issue is not present.

There is a but here, if I leave the filter-graph manager up and reload the source filter and then render the source filter pins the drag and drop problem is present.

Maybe it is not CoUninitialize at all.
My head hurts, :).


Thanks

[IanB]

What I'm wondering is if a call to CoInitialize returns RPC_E_CHANGED_MODE, does there still need to be a matching CoUnitialize call?Well it is an Error return, so one should expect that the use count not to be incremented so no CoUninit should be needed, but it certainly smacks of this being wrong and misleading.

Does anyone have a non-intrusive method to test if COM has been initialized. Really I would only like to call CoInitialize() if I know that it is going to return S_OK.

Perhaps the days of programing kindness is over and we should just enforce the "calling app must init com" rule and rip the CoInit/CoUnInit code out..

[foxyshadis]

Well, the fun way is to catch a crash, init com, and retry. I don't know anything about avisynth's host API, but I'm guessing the point of the first exception would be the AddRef() in the AVIFileSynth constructor?

[squid_80]

Maybe we should just ask Avery... :devil:

[IanB]

This thread, Avisynth - DirectShowSource "CoInitialize has not been called..", (http://forum.doom9.org/showthread.php?p=1020813#post1020813) prompts an idea that maybe we call CoInit/CoUninit in each DirectShowSource, AviSource, etc.

[Richard Berg]

Hmm, I don't like that. Not all source filters are part of the core, anyway.

I'm now of the opinion we shouldn't do it at all. Any app using the AviFile interface must already call CoInit or they will not be able to load Avisynth in the first place. The only real concern is apps creating their own IScriptEnvironments. The idea to put the call in CreateScriptEnvironment is an interesting one, but ultimately should not be necessary either. Avisynth is a library, not an application. It's not our job.

[IanB]

... It's not our job.I agree 100%. But! Plenty of applications screw COM and our poor users have next to no hope of getting there favourite software fixed, so we make a rod for our backs and add a hack here or there to help out.

The question ultimatly is how best to fashion the back rod.


Another thought might be to add a CoInitialize script verb for the really needy. Then any malady/solution is entirely in the users hands.

[Richard Berg]

Are there any programs that call CreateScriptEnvironment directly that are not open source? I know we allow this (avisynth.h exemption) but I didn't think any existed yet.

I'm not opposed to a CoInit verb if that's the only way to workaround broken programs.