Open Bug 16087 Opened 20 years ago Updated 7 years ago

API: Should proxy folder listeners when not on UI thread.

Categories

(MailNews Core :: Backend, defect)

x86
All
defect
Not set

Tracking

(Not tracked)

People

(Reporter: tonyr, Unassigned)

Details

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.
Status: NEW → ASSIGNED
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.
Severity: blocker → normal
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.
Target Milestone: M14 → M16
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
QA Contact: lchiang → nobody
Product: MailNews → Core
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
QA Contact: nobody → backend
Product: Core → MailNews Core
OS: Windows 98 → All
Is this still an issue in the newer rdf world
Flags: needinfo?
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.
Flags: needinfo?
You need to log in before you can comment on or make changes to this bug.