88135 - Duplicate summary files got created for IMAP special folders (-1, -2, etc) - sent-1.msf, drafts-1.msf, templates-1.msf

Status: NEW

(MailNews Core :: Networking: IMAP, defect)

Description

[Karen Huang]

Used 06-27-06-0.9.2 build

Dupicate summary files for special folders

If create new profile from today's build, check the summary files -- you will 
find that there are duplicate summary files for the special folders. 

You will find that there are additional summary files got created: sent-1.msf, 
drafts.msf, templates-1.msf

1) Create a new profile from today's build
2) Go to ImapMail directory to check the IMAP summary files.
3) Actual Results: All the summary files list correct except the special folders 
There are additional summary files (sent-1.msf, drafts.msf, templates-1.msf) got 
created which is not correct!!

Expected results: Should only create one summary file for each special folder.

Comment 1

[Karen Huang]

Nominating nsbeta1, adding keywords regression & Ccing Cavin.

Comment 2

[Karen Huang]

Problem is occurring on Windows & Mac platforms, didn't try on Linux platform 
yet!

Comment 3

[Karen Huang]

Ccing Bhuvan. This bug probably is from the fix of bug 23625.....

Comment 4

[scottputterman]

cc'ing bienvenu as well.

Comment 5

[Henrik Gemal]

yeps... still happening with build 20011018

I see "Drafts-1.msf" and "Sent-1.msf"

Comment 6

[leon.zhang]

still reproducable in trunk: 20020909
I can also see files : "Drafts-1.msf" and "Sent-1.msf"

Comment 7

[leon.zhang]

asigned to me

Comment 8

[leon.zhang]

I think code below may be related to this bug:

nsresult nsMsgDBFolder::CreateFileSpecForDB(const char *userLeafName, nsFileSpec
&path, nsIFileSpec **dbFileSpec)
{
  NS_ENSURE_ARG_POINTER(dbFileSpec);
  NS_ENSURE_ARG_POINTER(userLeafName);
  nsCAutoString proposedDBName(userLeafName);
  NS_MsgHashIfNecessary(proposedDBName);

  // (note, the caller of this will be using the dbFileSpec to call db->Open() 
  // will turn the path into summary spec, and append the ".msf" extension)
  //
  // we want db->Open() to create a new summary file
  // so we have to jump through some hoops to make sure the .msf it will
  // create is unique.  now that we've got the "safe" proposedDBName,
  // we append ".msf" to see if the file exists.  if so, we make the name
  // unique and then string off the ".msf" so that we pass the right thing
  // into Open().  this isn't ideal, since this is not atomic
  // but it will make do.
  proposedDBName+= SUMMARY_SUFFIX;
  path += proposedDBName.get();
  if (path.Exists()) 
  {
    path.MakeUnique();  *******************************************
    proposedDBName = path.GetLeafName();
  }
.....

   "path.MakeUnique(); " have created the filename such as "Drafts-1.msf" and
"Sent-1.msf". 

Comment 9

[leon.zhang]

Attachment: a patch for discussion

Comment 10

[leon.zhang]

I am very suprised  that there are many times of create the same ".smf" file
when create a new account and open it then.

Comment 11

[Henrik Gemal]

so perhaps also some kind of performance problem...?

Comment 12

[leon.zhang]

code below:
//Makes sure the database is open and exists.  If the database is valid then
//returns NS_OK.  Otherwise returns a failure error value.
nsresult nsImapMailFolder::GetDatabase(nsIMsgWindow *aMsgWindow)
{
  nsresult folderOpen = NS_OK;
  if (!mDatabase)
  {
    nsCOMPtr<nsIFileSpec> pathSpec;
    nsresult rv = GetPath(getter_AddRefs(pathSpec));
    if (NS_FAILED(rv)) return rv;

    nsCOMPtr<nsIMsgDatabase> mailDBFactory;

    rv = nsComponentManager::CreateInstance(kCImapDB, nsnull,
NS_GET_IID(nsIMsgDatabase), (void **) getter_AddRefs(mailDBFactory));
    if (NS_SUCCEEDED(rv) && mailDBFactory)
      folderOpen = mailDBFactory->OpenFolderDB(this, PR_TRUE, PR_FALSE,
getter_AddRefs(mDatabase));

    if(folderOpen == NS_MSG_ERROR_FOLDER_SUMMARY_MISSING || folderOpen ==
NS_MSG_ERROR_FOLDER_SUMMARY_OUT_OF_DATE)
      folderOpen = mailDBFactory->OpenFolderDB(this, PR_TRUE, PR_TRUE,
getter_AddRefs(mDatabase));

    if(mDatabase)
    {
      if(mAddListener)
        mDatabase->AddListener(this);
      UpdateSummaryTotals(PR_TRUE);
    }
  }
  return folderOpen;
}

there are two times of calling "mailDBFactory->OpenFolderDB".
I found that code above is related to create ".msf" files many times.

Comment 13

[David :Bienvenu]

Well, that'll certainly avoid us creating duplicate .msf files but I don't think
it's the right fix. There's a reason that makeUnique call is there - sometimes
two different mail folders can have the same .msf name, so we need to come up
with a unique name. This is a bit hard to explain, but sometimes folders can
have names that aren't legal file names on disk, and we need to have legal file
names on disk to create the .msf file. So, sometimes we need to map a folder
name to a different file name. Furthermore, sometimes two different folders can
map to the same file name, so we need to make the file name unique.

this isn't a recent regression, I don't think - we've had similar bugs around
for a long time. It's not even just IMAP special folders that cause this to
happen either. The reason there are two calls to OpenFolderDB is that the first
call failed, presumably because the file didn't exist on disk, and the second
call is to create the file on disk.

I don't have time to dig into this further right now, but I'll try to look at it
sometime soon. The real question to ask is why we ever get to this code when,
for example, Drafts.msf exists in the first place? was there an error opening
the db for some reason?

Comment 14

[leon.zhang]

I will try to analyze the codes related to this bug :)

About function:  nsImapMailFolder::GetDatabase(nsIMsgWindow *aMsgWindow)

When create a new account, mozilla will create "Inbox.msf", "Templates.msf",
"Sent.msf" and "drafts.msf" files, but the first time of calling: 
   folderOpen = mailDBFactory->OpenFolderDB(this, PR_TRUE, PR_FALSE,
getter_AddRefs(mDatabase));
  
The return value assgined to "folderOpen "  always is :
NS_MSG_ERROR_FOLDER_SUMMARY_MISSING

So process above  will lead to create create "Inbox.msf", "Templates.msf",
"Sent.msf" and "drafts.msf" files two times,although the file had been created
after called the "mailDBFactory->OpenFolderDB" at the first time.

-------------Discription above is not related to this bug closely,but should be
paid attention to :)

When begin load messgaes from the IMAP server after create a new account,mozilla
will check whether files "Inbox.msf" and "drafts.msf" have been created.
(
nsImapIncomingServer::PossibleImapMailbox -->
nsImapMailFolder::CreateClientSubfolderInfo --> 
  nsMsgDBFolder::CreateFileSpecForDB -->MakeUnique(create filename"Drafts-1.msf")
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  nsMailDatabase::Open -->nsMsgDatabase::OpenMDB -->myMDBFactory->CreateNewFile
(create file:"Drafts-1.msf")
)

So:
1) The focus should be "nsMsgDBFolder::CreateFileSpecForDB",becuase it create
nonexist filenames, such as"Drafts-1.msf".
2) Not create ".msf" files when create a new IMAP account,is it OK?
3) other solutions?

Comment 15

[Henrik Gemal]

leon zhang:
any update on this bug?

Comment 16

[Wayne Mery (:wsmwk)]

must be pretty rare (or gone) - no dups or almost no comments in 4 years.

Comment 17

[Wayne Mery (:wsmwk)]

david, the code of the proposed patch, just above "now, take the ".msf" off", was replaced by 1.300 remove nsFileSpec ...
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/mailnews/base/util&command=DIFF_FRAMESET&file=nsMsgDBFolder.cpp&rev2=1.300&rev1=1.299

so is the issue described by this bug still valid?

Comment 18

[David :Bienvenu]

the code is all changed on the trunk, but I've still seen this happen to some users...maybe not on the trunk, though.

Comment 19

[Wayne Mery (:wsmwk)]

interesting.  and my reading of the bug is there are no obvious symptoms visible in the UI, correct?  like two drafts folders?

Comment 20

[David :Bienvenu]

no obvious symptoms, because the .msf files represent the same URI, and because we use RDF in the folder pane, which maps them to the same resource, we only see one folder.

Comment 21

[Wayne Mery (:wsmwk)]

David, is this dead with de-rdf of folder pane?

Comment 22

[David :Bienvenu]

no, the de-rdf of the folder pane would more likely breath more life into this bug, i.e., make it more visible in the folder pane.