Open Bug 16087 Opened 20 years ago Updated 7 years ago
API: Should proxy folder listeners when not on UI thread
Create a new incoming server of type "none". Cretae a new account and set the incoming server to the one just created. Call server->GetRootFolder( &rootFolder); Call rootFolder->CreateSubFolder( "SubFolder"); and it always returns NS_ERROR_UNEXPECTED. Subsequent calls to rootFolder->GetChildNamed( "Any Name", &subFolder); cause a crash in some unidentifyable part of the JS engine.
Adding Seth ("none" man) and Scott (Local Mailman) Tony is working on the mail import stuff and is trying to create folders on an account he has set up through C++.. is it possible that the account/servers might not have been set up right? Scott?
This may have nothing to do with the server being "none". Here's what I see happening and I have no idea what to do to fix this! nsMsgLocalMailFolder::CreateSubFolder creates the folder, then call NotifyItemAdded which eventually get's to nsMsgMailSession::NotifyFolderItemAdded which enumerates through it's listeners calling OnItemAdded. One of these listeners dives into some code which hides the call stack in the debugger - xptcstubs.cpp - SharedStub is the new bottom of the call stack. Anyway, this gets into the JS engine where it eventually decides to abort on the following assertion: JS_ASSERT( me == CurrentThreadId()); AHA! The import code creates a thread to import mailboxes since that can be a very long process - calling CreateSubFolder from this other thread eventually get's back into the JS engine and dies. What is the answer? a) Should I not be in a thread? Importing can take anywhere from 2 seconds to 15 minutes depending upon size of daata and machine speed. Having a simple progress dialog visible while the import runs in another thread is the easiest and seemed the best thing to do. If this isn't so then how should this be accomplished? b) The CreateSubFolder code should be told whether or not to notify listeners? c) I have no clue!
I don't know the exact answer here. One of the folder listeners is coming from js. It sounds like there is some thread issue, but I don't know what it is. If we turned off notifications, we would need some way to notify the UI that a folder was added.
I mean to pipe in here, yeah.. the problem is that you cannot call JS in a JSContext that exists on a different thread than the one the JSContext was created in. I guess because some folder listeners could be (and are) implemented in JS, and declared on the UI thread, then any call that might set off a folder listener has to be set off on the UI thread. The other option we could do is allow folder listeners to exist on any thread, and when someone registers a folder listener, we store the thread ID... then when we go to call a folder listener, we proxy any calls whereever the folder listener's thread != the current thread. Tony, how hard do you think it would be to move your call to the UI thread? (I think the other option is a good thing to do anyway, but it could take alot of time to get right) I'm actually going to add Brendan & JBand to this one because I though of something interesting though maybe impossible: Brendan/JBand - do you think there would be any way to auto-proxy calls into a JSContext if the call comes from another thread? It would be specifically when you cross the XPConnect boundary...
Summary: API: nsIMsgFolder::CreateSubFolder fails when server is type "none" → API: Can't make some messenger API calls from non-UI thread
It will take some work for me to move all the folder calls into the UI thread but my guess is that it's probably not as much work as fixing all the API calls to work from any thread. In the interest of moving forward, unless I hear something in the next day or so that will fix this quickly I'll go ahead and re-work my code. (And I thought I was amost done!)
I just realized you can actually use this proxy code yourself if you want... what you basically do is create a proxied version of your object, then make calls to that object... the actual calls will happen on the thread you specify. see http://www.mozilla.org/projects/xpcom/Proxies.html What you should probably do is get the nsIEventQueue for the thread that created your thread...and use that as the first parameter to GetProxiedObject.. and then use PROXY_SYNC This way you don't have to redesign your code too much.
Proxies fixed the problem. Should this bug go away or should someone add some comments/doc somewhere to let developers know that they need to use proxy objects with the mailnews API's?
Summary: API: Can't make some messenger API calls from non-UI thread → API: API: Should proxy folder listeners when not on UI thread.
Target Milestone: M14
Glad to hear the proxies worked! It should definately be documented somewhere. I'll try to put something in http://www.mozilla.org/mailnews/arch/ The more I think about it though, I'd still like to fix the folder listener issue, so I'm changing the title of this bug.
Summary: API: API: Should proxy folder listeners when not on UI thread. → API: Should proxy folder listeners when not on UI thread.
ok, I talked to brendan and jband, and it looks like auto-proxification doesn't make sense here, so we should in fact be handling this as a special case.
Sounds like this is now a place where we can improve our code for use by outside developers, but that isn't a beta 1 stopper.
moving non-critical bugs to M20
Target Milestone: M16 → M20
reassign to putterman for folder listener stuff... sspitzer might also want to do this..
Assignee: alecf → putterman
Status: ASSIGNED → NEW
reassigning to sspitzer. I don't know if this is still relevant with new code.
Assignee: putterman → sspitzer
sorry for the spam. making bugzilla reflect reality as I'm not working on these bugs. filter on FOOBARCHEESE to remove these in bulk.
Assignee: sspitzer → nobody
Is this still an issue in the newer rdf world
Priority: P3 → --
All the folder operations probably need to be run on the main thread anyways; import running off main-thread is just a massive world of epic fail.
You need to log in before you can comment on or make changes to this bug.