Closed Bug 148043 Opened 22 years ago Closed 22 years ago

namespace prefs keep growing when checking for mail in multiple folders is on due to cached connections


(MailNews Core :: Networking: IMAP, defect)

Not set


(Not tracked)



(Reporter: mozilla-bugs, Assigned: mscott)


(Blocks 1 open bug)



(1 file)

My prefs.js currently contains


I keep seeing this on different RedHat machines with both Courier IMAP and MS
Exchange. From time to time I would clean up the prefs (e.g. leave only one
copy) and it'll start growing again. Seems to still be happening with recent
trunk builds.
Summary: namespace prefs keep growing. → namespace prefs keep growing.
do you have the check_all_folders pref set to on.
I do and I get this weird pref set to. On win2k
Severity: minor → normal
OS: Linux → All
Hardware: PC → All
Yes, I do have user_pref("mail.check_all_imap_folders_for_new", true) !
Also seen on Cyrus IMAP server.

I'm only seeing this on an slow machine 350Mhz P2. Not on my 900Mhz P3. What
machine are you on?
Summary: namespace prefs keep growing. → namespace prefs keep growing when "mail.check_all_imap_folders_for_new" is on
> What machine are you on?

Dual PPro-200; PII-266; PIII-450
Blocks: 160644
this bug is quite annoying. could be the source of some of the imap problems
users sometimes see, like selecting wrong folders, imap failures, etc...

I keep seeing this bug for the last year

this bug keeps cauing me to get "mailbox folder does not exists" IMAP error alerts
Severity: normal → major
the bug is both caused by having:
user_pref("mail.check_all_imap_folders_for_new", true)
and by setting "check folder on startup" via the folder property

it *have* something to do with using multiple connections, since my IMAP log 
shows something like:

1040 - * NAMESPACE (("INBOX." ".")) NIL (("" "."))
1040 - 3 OK Completed
1040 - 4 lsub "" "INBOX.*"
724  - * NAMESPACE (("INBOX." ".")) NIL (("" "."))
724  - 3 OK Completed
724  - 4 lsub "" "INBOX.*"
1224 - * NAMESPACE (("INBOX." ".")) NIL (("" "."))
1224 - 3 OK Completed
1224 - 4 lsub "" "INBOX.*"

so I'm actually having 3 connections to my IMAP just start Mozilla.

when having max_cached_connections set to 2 the pref gets set to:

which proofs my theory!
Summary: namespace prefs keep growing when "mail.check_all_imap_folders_for_new" is on → namespace prefs keep growing when checking for mail in multiple folders is on due to cached connections
Attached patch patch for testSplinter Review
Aleksey and henrik , please test it. It has work in my account.
I no longer have a setup to test this, sorry.
No matter. Thank you anyway. :-)
Looks like a great fix. I dont build mozilla myself, so I can test. But you
should seek review and get this baby in asap...

I have to turn off the "override namespace" because of this bug.
Attachment #107654 - Flags: review?(bienvenu)
Good job, Phillip.

Two things:

If the namespace is the same (which is what we're checking, right), we should
just return, no need to delete the old one and add the new one.

But more importantly, we shouldn't be issuing the namespace command for each

We could make a change like this:

  // mscott: VERIFY THIS CLAUSE!!!!!!!
    if (nameSpacesOverridable && !haveNameSpacesForHost)
--->>>>    m_hostSessionList->SetGotNamespacesForHost(GetImapServerKey(), PR_TRUE); 

then, we'd only get the namespace for the one connection, which is where we want
to end up, I think. Though now that I think about it, we'd have a race condition
between the connections, and the second/third/... connections might end up
without valid namespaces because the first connection hadn't retrieved it yet.
So, I think the best fix is the first one I described - if the new namespace
exists, just return.
Thx, David. I know what you mean.

But I still have a question.

If the originally created m_NamespaceList has more than one namespaces which is
same with what we want to add, should we let same namespaces exits in the
m_NamespaceList or delete them?(I prefer to delete any same ones)

For example, in this bug's condition, the original m_NamespaceList before we
sent 'namespace' command and add new namespace is "","","","","Shared
Folders/User/","Shared Folders/User/","Shared Folders/User/","Shared
Folders/User/", and when we add namespace "", we should delete all same
namespaces.(as in attachment 107654 [details] [diff] [review])

Of course ,if the original namespaceList is right, just return when we meet a
same namespace will work well.

Pls confirm it and I will add a new patch soon.
Phillip, I see your point, yes, we still want to delete all the namespaces that
come from the prefs when we get a namespace from the server so we don't want to
return before finishing the loop. So, I think your fix is the simplest way of
solving both problems.

So, why don't you just add a comment to your fix saying that? Something like:

// if we find existing namespace(s) that matches the new one, we'll just remove
// the old ones and let the new one get added when we've finished checking for
// matching namespaces or namespaces that came from prefs.

r=bienvenu, if you put a comment like that in there.
Comment on attachment 107654 [details] [diff] [review]
patch for test

actually, are we sure that GetPrefix will return a non-null pointer? Would we
have already crashed because the constructor does a PL_strdup of prefix?
Attachment #107654 - Flags: review?(bienvenu) → review+
Thanks, David. I will pay attention later.

To the failure of PL_strdup in constructor, I find it will still return (char*)0
as the m_Prefix when there are no space to malloc. So the call
PL_strcmp(ns->GetPrefix(),nspace->GetPrefix()) won't fail here. At other places
we must pay enough attention. Is it right? If not, pls tell me to have a change.

Thank you! David.
QA Contact: huang → gchan
Comment on attachment 107654 [details] [diff] [review]
patch for test


Pls add the comments when checking in according to David's suggestion.
Attachment #107654 - Flags: superreview+
Comment on attachment 107654 [details] [diff] [review]
patch for test

This is an annoying bug. The patch is small. It just make a strict check. The
risk is small.
Attachment #107654 - Flags: approval1.3?
Comment on attachment 107654 [details] [diff] [review]
patch for test

a=asa (on behalf of drivers) for checkin to 1.3final.
Attachment #107654 - Flags: approval1.3? → approval1.3+
Patch in.

=> Resolved
Closed: 22 years ago
Resolution: --- → FIXED
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.