Note: There are a few cases of duplicates in user autocompletion which are being worked on.

spin opening imap folder, new headers not downloaded

RESOLVED FIXED in Thunderbird 15.0

Status

MailNews Core
Networking: IMAP
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Bienvenu, Assigned: Bienvenu)

Tracking

Trunk
Thunderbird 15.0
x86_64
Windows 7

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
Created attachment 623681 [details] [diff] [review]
possible fix

I've gotten into a state where my imap drafts folder won't download new headers. I tracked it down to having bogus offline operations in the db, which led to errors trying to playback offline operations, which prevented us from trying to fetch new headers. I don't know what led to the bogus offline operations, but I was able to fix the offline playback code so that the bogus operations were removed and the folder was updated. W/o this patch, the user would need to repair the folder in order to get new headers downloaded.

I'd like to avoid duplicating the check for a bogus op type, which probably means wrapping it in a helper function. I'll try to do that real quick.
(Assignee)

Comment 1

5 years ago
Created attachment 623708 [details] [diff] [review]
proposed fix for review
Attachment #623681 - Attachment is obsolete: true
Attachment #623708 - Flags: review?(neil)

Comment 2

5 years ago
Comment on attachment 623681 [details] [diff] [review]
possible fix

>@@ -484,16 +484,21 @@ nsImapOfflineSync::ProcessAppendMsgOpera
>+  else
>+  {
>+    m_currentDB->RemoveOfflineOp(currentOp);
>+    ProcessNextOperation();
>+  }
Is this relevant to the bug?

>+          // remove operations with no type.
>+          if (!opType)
>+            m_currentDB->RemoveOfflineOp(currentOp);
>         // loop until we find the next db record that matches the current playback operation
>         while (currentOp && !(opType & mCurrentPlaybackOpType))
>         {
>           currentOp = nsnull;
>           ++m_KeyIndex;
>           if (m_KeyIndex < m_CurrentKeys.Length())
>             m_currentDB->GetOfflineOpForKey(m_CurrentKeys[m_KeyIndex],
>                                             false, getter_AddRefs(currentOp));
>           if (currentOp)
>+          {
>             currentOp->GetOperation(&opType);
>+            // remove operations with no type.
>+            if (!opType)
>+              m_currentDB->RemoveOfflineOp(currentOp);
>+          }
>         }
You can get rid of the duplication like this:
// loop until we find the next db record that matches the current playback operation
while (currentOp && !(opType & mCurrentPlaybackOpType))
{
  // remove operations with no type.
  if (!opType)
    m_currentDB->RemoveOfflineOp(currentOp);
  currentOp = nsnull;
  ++m_KeyIndex;
  if (m_KeyIndex < m_CurrentKeys.Length())
    m_currentDB->GetOfflineOpForKey(m_CurrentKeys[m_KeyIndex],
                                    false, getter_AddRefs(currentOp));
  if (currentOp)
  {
    currentOp->GetOperation(&opType);
    // remove operations with no type.
    if (!opType)
      m_currentDB->RemoveOfflineOp(currentOp);
  }
}
(Assignee)

Comment 3

5 years ago
(In reply to neil@parkwaycc.co.uk from comment #2)
> Comment on attachment 623681 [details] [diff] [review]
> possible fix
> 
> >@@ -484,16 +484,21 @@ nsImapOfflineSync::ProcessAppendMsgOpera
> >+  else
> >+  {
> >+    m_currentDB->RemoveOfflineOp(currentOp);
> >+    ProcessNextOperation();
> >+  }
> Is this relevant to the bug?

yes, in the sense that it also contributed to the spin opening the folder. The methods that process operations need to call ProcessNextOperation if they don't run a url, or we'll spin, and the other methods (e.g., ProcessFlagOperations) do so.

> You can get rid of the duplication like this:
> // loop until we find the next db record that matches the current playback
> operation
> while (currentOp && !(opType & mCurrentPlaybackOpType))
> {
>   // remove operations with no type.
>   if (!opType)
>     m_currentDB->RemoveOfflineOp(currentOp);
>   currentOp = nsnull;
>   ++m_KeyIndex;
>   if (m_KeyIndex < m_CurrentKeys.Length())
>     m_currentDB->GetOfflineOpForKey(m_CurrentKeys[m_KeyIndex],
>                                     false, getter_AddRefs(currentOp));
>   if (currentOp)
>   {
>     currentOp->GetOperation(&opType);
>     // remove operations with no type.
>     if (!opType)
>       m_currentDB->RemoveOfflineOp(currentOp);
>   }
> }

that duplicates the call to RemoveOfflineOp at the top and bottom of the loop, unless I'm missing something.

Comment 4

5 years ago
(In reply to David Bienvenu from comment #3)
> (In reply to comment #2)
> > You can get rid of the duplication like this:
> > [snip failed attempt]
> that duplicates the call to RemoveOfflineOp at the top and bottom of the
> loop, unless I'm missing something.
Yeah, I copied and pasted instead of cut and pasted. Let me try again:
// loop until we find the next db record that matches the current playback operation
while (currentOp && !(opType & mCurrentPlaybackOpType))
{
  // may as well remove operations with no type while we're here
  if (!opType)
    m_currentDB->RemoveOfflineOp(currentOp);
  currentOp = nsnull;
  ++m_KeyIndex;
  if (m_KeyIndex < m_CurrentKeys.Length())
    m_currentDB->GetOfflineOpForKey(m_CurrentKeys[m_KeyIndex],
                                    false, getter_AddRefs(currentOp));
  if (currentOp)
    currentOp->GetOperation(&opType);
}
(Assignee)

Comment 5

5 years ago
Created attachment 626981 [details]
fix addressing comments

yes, that's better, and should work.
Attachment #623708 - Attachment is obsolete: true
Attachment #623708 - Flags: review?(neil)
Attachment #626981 - Flags: review?(neil)

Updated

5 years ago
Attachment #626981 - Flags: review?(neil) → review+
(Assignee)

Comment 6

5 years ago
fixed on trunk - http://hg.mozilla.org/comm-central/rev/3b21da7e9eff

don't know if anyone else ever saw this, or how to recreate it (the workaround would have been to repair the folder) but maybe this will help others...
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 15.0
You need to log in before you can comment on or make changes to this bug.