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

NEW
Unassigned

Status

19 years ago
5 years ago

People

(Reporter: tonyr, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

19 years ago
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.

Updated

19 years ago
Status: NEW → ASSIGNED

Comment 1

19 years ago
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?
(Reporter)

Comment 2

19 years ago
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!

Comment 3

19 years ago
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.

Comment 4

19 years ago
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
(Reporter)

Comment 5

19 years ago
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!)

Comment 6

19 years ago
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.
(Reporter)

Updated

19 years ago
Severity: blocker → normal
(Reporter)

Comment 7

19 years ago
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?

Updated

19 years ago
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

Comment 8

19 years ago
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.

Updated

19 years ago
Summary: API: API: Should proxy folder listeners when not on UI thread. → API: Should proxy folder listeners when not on UI thread.

Comment 9

19 years ago
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.

Updated

19 years ago
Target Milestone: M14 → M16

Comment 10

19 years ago
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.

Comment 11

19 years ago
moving non-critical bugs to M20
Target Milestone: M16 → M20

Comment 12

18 years ago
reassign to putterman for folder listener stuff... sspitzer might also want to
do this..
Assignee: alecf → putterman
Status: ASSIGNED → NEW

Comment 13

17 years ago
reassigning to sspitzer.  I don't know if this is still relevant with new code.
Assignee: putterman → sspitzer

Updated

16 years ago
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

Updated

10 years ago
QA Contact: nobody → backend
(Assignee)

Updated

10 years ago
Product: Core → MailNews Core

Updated

9 years ago
OS: Windows 98 → All

Comment 15

5 years ago
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.