Open Bug 589870 Opened 14 years ago Updated 2 years ago

Too large number for downloaded messages displayed in status bar after automatic e-mail download, for example, 4294967294==0xFFFFFFFE==-2 of 32bits signed integer

Categories

(Thunderbird :: Mail Window Front End, defect)

x86
All
defect

Tracking

(Not tracked)

People

(Reporter: himorin, Unassigned)

References

(Depends on 1 open bug)

Details

Attachments

(10 files, 3 obsolete files)

Attached image screenshot
I cannot figure out how to reproduce this, but I have seen this three or more times today.

Sometimes, TB show a message like attached screen shot in its status bar.
> 4,294,967,220 e-mail downloaded (or something; in English)
I have set automatic e-mail download for two accounts, but I don't have so much mails. Of cource, little or no e-mail have downloaded just before the message.


version: Mozilla/5.0 (Windows; U; Windows NT 6.1; ja; rv:1.9.2.9pre) Gecko/20100822 Lanikai/3.1.3pre
Nothing in Tools -> Error console I suppose ?
(In reply to comment #1)
> Nothing in Tools -> Error console I suppose ?

Two messages.

Error: uncaught exception: [Exception... "Component returned failure code: 0x8055000a [nsIMsgIncomingServer.getNewMessages]"  nsresult: "0x8055000a (<unknown>)"  location: "JS frame :: chrome://messenger/content/mailWindowOverlay.js :: GetNewMsgs :: line 2346"  data: no]

Error: [Exception... "Component returned failure code: 0x80004001 (NS_ERROR_NOT_IMPLEMENTED) [nsIRequest.name]"  nsresult: "0x80004001 (NS_ERROR_NOT_IMPLEMENTED)"  location: "JS frame :: file:///C:/user/thunderbird/thunderbird-3.1.3pre.ja.win32.20100822/components/nsLoginManager.js :: anonymous :: line 315"  data: no]
Source file: file:///C:/user/thunderbird/thunderbird-3.1.3pre.ja.win32.20100822/components/nsLoginManager.js
line: 315

I use 'Enigmail' in this profile, but I hope these two are not related on this extension.

Masayuki (at Mozilla Japan) told me that connection logs might help to solve. Is it in need?
Those error messages aren't enigmail related. Yes they might help Do you know how to make the logs ?
(In reply to comment #3)
> Those error messages aren't enigmail related. Yes they might help Do you know
> how to make the logs ?

Which loglevel do you need?
NSPR_LOG_MODULES=smtp:3,pop3:3,timestamp is worth?
5 :-D we need pop or imap depending on what you are using. Timestamp is unnecessary at the moment.
(In reply to comment #5)
> 5 :-D we need pop or imap depending on what you are using. Timestamp is
> unnecessary at the moment.

Ah, yeah. I mean 5 :-)
Log without email body (with email body, file has more than 22000 lines..)
Himorin what's the ratio , ie you download one and how much more are displayed ?
(In reply to comment #8)
> Himorin what's the ratio , ie you download one and how much more are displayed
> ?

ratio,, occur / e-mail downloads?
I think,, hrm. I set automatic (periodical) download by 10 min, and I think I see such message once in two or three hours. So, might be 1/10 to 1/20 or so?
From the referred to Bug 594315 it seems that this is a typical underflow situation where -1 (or lower) is interpreted as integer, resulting in an extraordinarily high number. This might occur, for instance, if the calculation of "messages downloaded" is the total downloaded minus the filtered away messages (the ones that are auto-set to read).

An obvious code-fix would be to check for this underflow situation and consider negative outcomes as zero.
Confirming based on the numbers of duplicates.
Status: UNCONFIRMED → NEW
Ever confirmed: true
blocking-thunderbird3.2: --- → ?
Adding attachment to confirm that the bug is still present in Thunderbird 3.1.5 which was released yesterday. As an added remark: the number displayed "4297967295" is always the same on my machine, it seems. 

The oddity occurs roughly once or twice a day on a configuration that receives about 3500 messages per day and a total combined mailbox size of slightly over 4GB.
Still exist in Thunderbird/3.1.9. 

I think the bug is worth a fix. It confuses users and makes users feel TB is not that reliable.
blocking-thunderbird3.2: ? → ---
Flags: wanted-thunderbird+
@Dingding: I agree, it is indeed confusing. 3.1.10 is out, meanwhile, which still shows me "4297967295 messages downloaded". 

I made the following observation: it seems to happen only when you have filters running and the net result in your inbox from a bunch of incoming messages is zero, and others have been moved and sometimes also been marked "read". 

Maybe someone else can confirm this observation?
Is that your build number?!?!

I'm getting "4294967294 messages downloaded"
No, it is not a build number. It is 2 to the power of 32 minus a small number, i.e. 4294967296 - 2 in your case. It's a sypical overflow situation, where somewhere in the Thunderbird code the amount of new messages is calculated. I think it deducts the number of spam and/or filtered and deleted messages (why, I don't know) and perhaps even deducts moved and marked read messsges. If this number exceeds the number of untouched messages, you receive this strange behavior. 

The number then underflows below zero, which is represented in computer memory as 4294967295 for -1. Since the code expects a pssitive number, it ignores the so-called sign-bit, resulting in a high number to be represented. 

While it is kind of clear that this is happening, I have no idea where in the code this bug is, otherwise I would have fixed it myself. Someone with more knowledge of the codebase should someday take a look at this.
Still occurs in Thunderbird Version 7.0.1

 429496795 messages downloaded

It happened immediately after sending a message.
@Abel,
I know how to fix this, I am a programmer. but not a TB programmer.
It appears that there is a variable 'int qtydownloads' that is being set to either 0 or -1 on startup. When there is a check for the number of emails downloaded, it checks to see if greater than '0' but it may be -1. Then there is a print formatting routine that prints 'unsigned int values' so -1 is in fact 4294967295.

A quick fix will be to fix the formatting code that shows the result such that if the number to be printed is less than 0, then fix the number before trying to display it. Hope this helps.

Know what, how can I get the source??
This page gives info on downloading and building the source:

https://developer.mozilla.org/En/Simple_Thunderbird_build

You can also browse the source here:

http://mxr.mozilla.org/comm-central/

For new mail notifications you probably want to start looking in nsMessenger*Integration:

http://mxr.mozilla.org/comm-central/find?text=&string=nsMessenger.*Integration

(In reply to Colin B Maharaj from comment #24)
> It appears that there is a variable 'int qtydownloads' that is being set to
> either 0 or -1 on startup.

Given that doesn't appear in the source, I'm guessing you're just theorising. I very much doubt it would be something like that. I suspect a more subtle bug which is calculating counts wrongly.
(In reply to Abel Braaksma from comment #20)
> 
> I made the following observation: it seems to happen only when you have
> filters running and the net result in your inbox from a bunch of incoming
> messages is zero, and others have been moved and sometimes also been marked
> "read". 
> 
> Maybe someone else can confirm this observation?

Same here. Have filters running, recieved an e-mail, which was moved to another folder, after which thunderbird displayed this large number. However, I haven't disabled my filters, so I don't know if that would make any difference.
Hi.
I expirienced the same in 10.0.1. I traced through the source and found in 
http://mxr.mozilla.org/comm-central/source/mailnews/local/src/nsPop3Sink.cpp

346   PRInt32 numNewMessagesInFolder;
347   // if filters have marked msgs read or deleted, the num new messages count
348   // will go negative by the number of messages marked read or deleted,
349   // so if we add that number to the number of msgs downloaded, that will give
350   // us the number of actual new messages.
351   m_folder->GetNumNewMessages(false, &numNewMessagesInFolder);
352   m_numNewMessages -= (m_numNewMessagesInFolder  - numNewMessagesInFolder);
353   m_folder->SetNumNewMessages(m_numNewMessages); // we'll adjust this for spam later

The m_numNewMessagesInFolder is set via m_folder->GetNumNewMessages at another place. m_numNewMessages is given from external calls. So we check if there was a change and subtract the difference from m_numNewMessages. While I do not know the exact interpretation of the variables (since I did not read the full source), line 353 looks inconsistent to me: If m_numNewMessages is supposed to contain the overall number of new messages it does not make sense to put in in m_folders counter. Moreover, it might be a good idea to update m_numNewMessagesInFolder after correcting m_numNewMessages to avoid double correction.
(In reply to Eijk from comment #27)
> Hi.
> I expirienced the same in 10.0.1. I traced through the source and found in 
> http://mxr.mozilla.org/comm-central/source/mailnews/local/src/nsPop3Sink.cpp
> 
> 346   PRInt32 numNewMessagesInFolder;
> 347   // if filters have marked msgs read or deleted, the num new messages
> count
> 348   // will go negative by the number of messages marked read or deleted,
> 349   // so if we add that number to the number of msgs downloaded, that
> will give
> 350   // us the number of actual new messages.
> 351   m_folder->GetNumNewMessages(false, &numNewMessagesInFolder);
> 352   m_numNewMessages -= (m_numNewMessagesInFolder  -
> numNewMessagesInFolder);
> 353   m_folder->SetNumNewMessages(m_numNewMessages); // we'll adjust this
> for spam later

Would you like to propose a patch Eijk ?
(In reply to Ludovic Hirlimann [:Usul] from comment #28)
> Would you like to propose a patch Eijk ?

There are many interdependencies in the code, so I am not sure what to propose without creating other problems.
There are around four different counters for the new messages, which are updated independently, so one really has to take care to do it the right way.
However, I cannot say yet what is the right way, since I do not understand all dependencies. I could guess what might work, but I think it is better if someone how knows the code does a patch.
I have the same problem("4294967295 messages download"). version 10.0.2
I need to be honest, previous beta versions were not showing me this error again but after I updated Thunderbird to version 11.0 the problem presented again today as you can see from the screenshot.

[im]http://yogem.com/thunderbird.PNG[/im]

And the exact wrong count is of 4294967295 downloaded messages!
If it is of any use. 4294967295 is exactly 4GB in bytes!

Below is a list of files containing this exact string (Find And Replace (FAR) is SO handy) relative to the decompressed tar.bz2 source archive:


comm-release\calendar\base\content\dialogs\calendar-print-dialog.js
comm-release\mailnews\base\util\nsMsgKeySet.cpp
comm-release\mozilla\browser\devtools\webconsole\HUDService.jsm
comm-release\mozilla\content\canvas\test\webgl\conformance\typedarrays\array-unit-tests.html
comm-release\mozilla\content\html\content\test\reflect.js
comm-release\mozilla\gfx\angle\src\compiler\glslang_lex.cpp
comm-release\mozilla\gfx\angle\src\compiler\preprocessor\new\pp_lex.cpp
comm-release\mozilla\gfx\cairo\libpixman\src\pixman-compiler.h
comm-release\mozilla\image\test\unit\test_imgtools.js
comm-release\mozilla\ipc\chromium\src\base\string_util_unittest.cc
comm-release\mozilla\ipc\chromium\src\base\third_party\nspr\prtypes.h
comm-release\mozilla\js\src\jsarray.cpp
comm-release\mozilla\js\src\jsdtoa.cpp
comm-release\mozilla\js\src\jsstdint.h
comm-release\mozilla\js\src\jstracer.cpp
comm-release\mozilla\js\src\jit-test\tests\basic\bug563125.js
comm-release\mozilla\js\src\jit-test\tests\basic\bug657245.js
comm-release\mozilla\js\src\jit-test\tests\basic\bug691299-regexp.js
comm-release\mozilla\js\src\jit-test\tests\basic\setprop-with-index.js
comm-release\mozilla\js\src\jit-test\tests\basic\testBitopWithConstan.js
comm-release\mozilla\js\src\jit-test\tests\basic\testInitSingletons.js
comm-release\mozilla\js\src\jit-test\tests\jaeger\bug577705.js
comm-release\mozilla\js\src\jit-test\tests\jaeger\bug587431.js
comm-release\mozilla\js\src\jit-test\tests\jaeger\bug652590.js
comm-release\mozilla\js\src\jit-test\tests\jaeger\floatTypedArrays.js
comm-release\mozilla\js\src\jit-test\tests\jaeger\normalIntTypedArrays.js
comm-release\mozilla\js\src\jsapi-tests\testIndexToString.cpp
comm-release\mozilla\js\src\tests\ecma\Array\15.4.1.2.js
comm-release\mozilla\js\src\tests\ecma\Array\15.4.2.2-1.js
comm-release\mozilla\js\src\tests\ecma\LexicalConventions\7.7.3-1.js
comm-release\mozilla\js\src\tests\ecma\TypeConversion\9.3.1-3.js
comm-release\mozilla\js\src\tests\ecma\TypeConversion\9.5-2.js
comm-release\mozilla\js\src\tests\ecma\TypeConversion\9.6.js
comm-release\mozilla\js\src\tests\ecma_3\Array\regress-322135-01.js
comm-release\mozilla\js\src\tests\ecma_3\Array\regress-322135-02.js
comm-release\mozilla\js\src\tests\ecma_3\Array\regress-322135-03.js
comm-release\mozilla\js\src\tests\ecma_3\Array\regress-322135-04.js
comm-release\mozilla\js\src\tests\js1_5\Array\regress-157652.js
comm-release\mozilla\js\src\tests\js1_5\Array\regress-465980-02.js
comm-release\mozilla\js\src\tests\src\jstests.jar
comm-release\mozilla\js\src\vm\String.h
comm-release\mozilla\netwerk\streamconv\converters\parse-ftp\V-VMS-mix.in
comm-release\mozilla\nsprpub\pr\include\prtypes.h
comm-release\mozilla\other-licenses\nsis\Contrib\CityHash\cityhash\stdint.h
comm-release\mozilla\security\nss\cmd\certcgi\certcgi.c
comm-release\mozilla\security\nss\lib\freebl\mpi\mpi.h
comm-release\mozilla\security\nss\lib\libpkix\pkix_pl_nss\system\pkix_pl_common.c
comm-release\mozilla\toolkit\xre\nsNativeAppSupportOS2.cpp
comm-release\mozilla\toolkit\xre\nsNativeAppSupportWin.cpp
comm-release\mozilla\tools\trace-malloc\tmfrags.c
comm-release\mozilla\xpcom\ds\nsVariant.cpp
comm-release\mozilla\xpcom\tests\TestStrings.cpp
Sorry, off by a byte :)
(In reply to Avi Kohn from comment #32)
> If it is of any use. 4294967295 is exactly 4GB in bytes!

It's 2^32-1, or in other words, it's what you get if you interpret -1 as an unsigned 32bit integer. As comment #12, comment #22 and comment #24 already discussed. So the printed value is in fact the result of some broken computation, not a value copied verbatim from some constant or similar. Therefore a listing of occurrences likely won't help. For the future, quoting mxr URLs for searches might be better: http://mxr.mozilla.org/comm-central/search?string=4294967295 is shorter than the list you pasted.
I understand that my comment will probably not be very helpful but anyway.... 

Same problem on my Thunderbird 11.0.1, Windows 7 Ultimate SP1
Unfortunately I could not determine the circumstances which lead to this symptoms.
I also saw this with the version before the last update (12.0.1 is now but I do not remember what it was before): However with the number 294 in the end, so 2^32-2.

What I did is automatically checking for new messages and deleting some of them relatively fast. Also automatic junk detection is turned on. They are then moved to the trash folder and purged upon exit.

This might have confused the programm to subtract one or two too many.
I just observed Thunderbird 16.0.2 display "4294967294 messages downloaded" in the lower left corner status frame.  This is an Ubuntu 12.04.1 LTS build.  I also have many message filters that move incoming messages into various folders.
Still present in 17.0.2.

It would appear we have two errors here:

1. The code displaying the number evidently interprets the value as an unsigned integer when the calculation treats it as signed.

2. Surely the number of messages downloaded should be a starting point for other calculations, not derived from adding and subtracting messages processed in various ways - that's just complicating things unnecessarily.
I think I can see the problem - line 351 above (320 in the comm-central code now) - has -= when if it's supposed to be adding the difference back. It should surely be += unless the programmer has obfuscated the code by putting the operands to the subtraction in reverse order.
Or, as a patch:

diff -uNr comm-release-old/mailnews/local/src/nsPop3Sink.cpp comm-release/mailnews/local/src/nsPop3Sink.cpp
--- comm-release-old/mailnews/local/src/nsPop3Sink.cpp	2013-01-07 20:43:36.000000000 +0000
+++ comm-release/mailnews/local/src/nsPop3Sink.cpp	2013-02-27 00:57:50.365298009 +0000
@@ -317,7 +317,7 @@
   // so if we add that number to the number of msgs downloaded, that will give
   // us the number of actual new messages.
   m_folder->GetNumNewMessages(false, &numNewMessagesInFolder);
-  m_numNewMessages -= (m_numNewMessagesInFolder  - numNewMessagesInFolder);
+  m_numNewMessages += (m_numNewMessagesInFolder  - numNewMessagesInFolder);
   m_folder->SetNumNewMessages(m_numNewMessages); // we'll adjust this for spam later
   if (!filtersRun && m_numNewMessages > 0)
   {
>    // so if we add that number to the number of msgs downloaded, that will
> give
>    // us the number of actual new messages.
>    m_folder->GetNumNewMessages(false, &numNewMessagesInFolder);
> -  m_numNewMessages -= (m_numNewMessagesInFolder  - numNewMessagesInFolder);
> +  m_numNewMessages += (m_numNewMessagesInFolder  - numNewMessagesInFolder);

I don't think this will work, because we want to subtract off the number of messages we thought there were, and then add the number of messages we now think there are, which is what we're doing with the original version.

(The original version is:
  m_numNewMessages -= (m_numNewMessagesInFolder  - numNewMessagesInFolder);
which we can re-write as:
  m_numNewMessages = m_numNewMessages - (m_numNewMessagesInFolder  - numNewMessagesInFolder);
which is the same as:
  m_numNewMessages = m_numNewMessages - m_numNewMessagesInFolder  + numNewMessagesInFolder;
which I think is what we want…)

Also, I see this occasionally, and I don't have any POP3 accounts…  ;)

Thanks,
Blake.
(In reply to Blake Winton (:bwinton) from comment #42)
> which is the same as:
>  m_numNewMessages =
>  m_numNewMessages - m_numNewMessagesInFolder + numNewMessagesInFolder;

Cooment for the equation.
> http://mxr.mozilla.org/comm-central/source/mailnews/local/src/nsPop3Sink.cpp#315
> 315   // if filters have marked msgs read or deleted, the num new messages count
> 316   // will go negative by the number of messages marked read or deleted,
> 317   // so if we add that number to the number of msgs downloaded, that will give
> 318   // us the number of actual new messages.

An initial phenomenon of incorrect new mail count was one like next;
  When N new mails arrive in empty Inbox,
  if all N mails are moved to other folder by Message filter,
  or if all N mails are moved to Junk by Junk filter,
  Biff says "new mail count==N", but no mail exists in Inbox.
Delta by message filter looks already counted.
 
When following occurs at same time, will above equation produce always ZERO or positive value?
- new mail is moved to Junk folder or deleted by Junk filter
- new mail is moved to other folder from Inbox by message filter,
  then the moved mail is moved to Junk folder or deleted by Junk filter

Currently, Message filter has option of "(after clasification) or not".
Is value by above equation affected by (after clasification) or not(==before clasification)?
(before : Message filter runs, then Junk Filter runs)
(after  : Junk filter runs,    then Messae filter runs == new by enhancement)
(In reply to Blake Winton (:bwinton) from comment #42)
> >    // so if we add that number to the number of msgs downloaded, that will
> > give
> >    // us the number of actual new messages.
> >    m_folder->GetNumNewMessages(false, &numNewMessagesInFolder);
> > -  m_numNewMessages -= (m_numNewMessagesInFolder  - numNewMessagesInFolder);
> > +  m_numNewMessages += (m_numNewMessagesInFolder  - numNewMessagesInFolder);
> 
> I don't think this will work, because we want to subtract off the number of
> messages we thought there were, and then add the number of messages we now
> think there are, which is what we're doing with the original version.
> 
> (The original version is:
>   m_numNewMessages -= (m_numNewMessagesInFolder  - numNewMessagesInFolder);
> which we can re-write as:
>   m_numNewMessages = m_numNewMessages - (m_numNewMessagesInFolder  -
> numNewMessagesInFolder);
> which is the same as:
>   m_numNewMessages = m_numNewMessages - m_numNewMessagesInFolder  +
> numNewMessagesInFolder;
> which I think is what we want…)
> 
> Also, I see this occasionally, and I don't have any POP3 accounts…  ;)
> 
> Thanks,
> Blake.

If this code is only executed for POP3, either it is duplicated for IMAP or it's not where the problem occurs. I don't know enough about this really, but assumed we had been directed to the right piece of code, and read the comment which talked about adding back what had been moved to restore the correct count, and noted that the code then subtracted a difference rather than adding it. As I don't know what the m_ on the beginning of the variable means I can't be sure what this does.

As I wrote in 39 (2) above, all this messing around with the count is a mystery to me. Surely the number of messages downloaded is the number downloaded. How should that be affected by subsequent deletion or moving? They were still downloaded and that's what I'd expect to see in the status bar. The number marked as unread in a folder is a different matter. Surely the number downloaded is where the other calculations might start, but should remain unchanged until the next download.
No, my patch doesn't work. It removes the "messages downloaded" from the status bar altogether after a download.
Correction: after an hour of running it's back! So clearly not the problem at all.
This works for me:

diff -uNr comm-release-old/mailnews/local/src/nsPop3Sink.cpp comm-release/mailnews/local/src/nsPop3Sink.cpp
--- comm-release-old/mailnews/local/src/nsPop3Sink.cpp	2013-01-07 20:43:36.000000000 +0000
+++ comm-release/mailnews/local/src/nsPop3Sink.cpp	2013-02-27 22:56:13.303914805 +0000
@@ -391,7 +391,7 @@
 #endif
   nsCOMPtr<nsIPop3Service> pop3Service(do_GetService(NS_POP3SERVICE_CONTRACTID1, &rv));
   NS_ENSURE_SUCCESS(rv, rv);
-  pop3Service->NotifyDownloadCompleted(m_folder, m_numNewMessages);
+  pop3Service->NotifyDownloadCompleted(m_folder, (uint32_t)m_numMsgsDownloaded);
   return NS_OK;
 }
Well, almost. It seems if the mail download loop completes m_numMsgsDownloaded is one greater than actual value, so it needs correcting and storing before use, So I'm now testing the following and will report later:

diff -uNr comm-release-old/mailnews/local/src/nsPop3Sink.cpp comm-release/mailnews/local/src/nsPop3Sink.cpp
--- comm-release-old/mailnews/local/src/nsPop3Sink.cpp	2013-01-07 20:43:36.000000000 +0000
+++ comm-release/mailnews/local/src/nsPop3Sink.cpp	2013-02-28 13:55:04.887755620 +0000
@@ -312,6 +312,11 @@
   bool filtersRun;
   m_folder->CallFilterPlugins(nullptr, &filtersRun); // ??? do we need msgWindow?
   int32_t numNewMessagesInFolder;
+  // bug 589870 - need to store actual number of messages downloaded before
+  // destroying record. If all mail downloaded control variable will be one
+  // more than total, so need to check and correct.
+  uint32_t numDownloadedForStatusMsg;
+  numDownloadedForStatusMsg = (m_numMsgsDownloaded > m_numNewMessages)?(uint32_t)m_numNewMessages:(uint32_t)m_numMsgsDownloaded;
   // if filters have marked msgs read or deleted, the num new messages count
   // will go negative by the number of messages marked read or deleted,
   // so if we add that number to the number of msgs downloaded, that will give
@@ -391,7 +396,7 @@
 #endif
   nsCOMPtr<nsIPop3Service> pop3Service(do_GetService(NS_POP3SERVICE_CONTRACTID1, &rv));
   NS_ENSURE_SUCCESS(rv, rv);
-  pop3Service->NotifyDownloadCompleted(m_folder, m_numNewMessages);
+  pop3Service->NotifyDownloadCompleted(m_folder, numDownloadedForStatusMsg);
   return NS_OK;
 }
No, that puts us back to 4294967295 for one message downloaded and deleted. How intriguing!
Interesting; m_numNewMessages seems to be negative even before the filter plug-ins are called in these circumstances. It also appears the m_numNewMessages -= (m_numNewMessagesInFolder  - numNewMessagesInFolder); line does nothing, so the two messages in folder counts appear to be equal (something else has already updated it?).

I'm left with the following hack for now, which at least displays the right count in normal circumstances. However, no count is displayed on the initial start-up of thunderbird, nor on a manual download, so I suspect the problem is deeper, but I lack a sufficient overview to know where to delve.

This fixes the current bug, but is probably only treating one symptom of something else:
+++ comm-release/mailnews/local/src/nsPop3Sink.cpp	2013-02-28 22:43:24.948445424 +0000
@@ -309,6 +309,12 @@
   nsresult rv = ReleaseFolderLock();
   NS_ASSERTION(NS_SUCCEEDED(rv),"folder lock not released successfully");
 
+  // bug 589870 - need to store actual number of messages downloaded before
+  // changing record. If mail downloaded control variable will be one
+  // more than total, so need to check and correct.
+  uint32_t numDownloadedForStatusMsg;
+  numDownloadedForStatusMsg = (m_numMsgsDownloaded > 0)?(uint32_t)m_numMsgsDownloaded - 1:0;
+
   bool filtersRun;
   m_folder->CallFilterPlugins(nullptr, &filtersRun); // ??? do we need msgWindow?
   int32_t numNewMessagesInFolder;
@@ -391,7 +397,7 @@
 #endif
   nsCOMPtr<nsIPop3Service> pop3Service(do_GetService(NS_POP3SERVICE_CONTRACTID1, &rv));
   NS_ENSURE_SUCCESS(rv, rv);
-  pop3Service->NotifyDownloadCompleted(m_folder, m_numNewMessages);
+  pop3Service->NotifyDownloadCompleted(m_folder, numDownloadedForStatusMsg);
   return NS_OK;
 }
I've read the procedure for submitting a patch and I've tried to contact the sub-module owner, but have had no reply. I've looked at the procedure for accessing the try server, but it looks too heavyweight for someone who does not wish to become a regular Mozilla developer.

I'm sorry, but complying with your procedures is too onerous for the casual helper. All I want to do is suggest a possible remedy for the problem. Here it is, if you want it. If not, I'll submit it downstream to the distribution I use and they can try it out.

Sorry again if that's not as helpful as you would like. I'm doing my best in the circumstances.
Comment on attachment 722205 [details] [diff] [review]
Displays the correct number of Downloads when a filter moves or marks a message

(In reply to firefox.bugs from comment #51)
> I've read the procedure for submitting a patch and I've tried to contact the
> sub-module owner, but have had no reply. I've looked at the procedure for
> accessing the try server, but it looks too heavyweight for someone who does
> not wish to become a regular Mozilla developer.

Hmm.  The process should really be easier than that.  I wonder if we're not documenting it well enough (or in the right places).

> I'm sorry, but complying with your procedures is too onerous for the casual
> helper. All I want to do is suggest a possible remedy for the problem.
> Sorry again if that's not as helpful as you would like. I'm doing my best in
> the circumstances.

That's wonderfully helpful, and we really do appreciate your work here!  I'm sorry you're finding the procedures hard to follow, but it might make you happier to know that we've got someone working on making them easier.  :)

In the mean time, I'm going to help push this forward by setting the review flag on this patch to ask Standard8 to review it.  I've also asked Irving for feedback, in case Standard8 is too busy, and he wants to steal the review.  I'm sure they'll get to this as soon as they can!

Thanks,
Blake.
Attachment #722205 - Flags: review?(mbanner)
Attachment #722205 - Flags: feedback?(irving)
Comment on attachment 722205 [details] [diff] [review]
Displays the correct number of Downloads when a filter moves or marks a message

Review of attachment 722205 [details] [diff] [review]:
-----------------------------------------------------------------

As Blake said, thanks for looking into this, and I agree that the procedure for producing a "perfect" patch can be annoying. We're happy to do that part for you.

You've made me wonder what's going on to make m_numMsgsDownloaded be one more than it should be (when it isn't zero). Before I give r+ on this I'm going to spend some time looking at the code leading up to http://mxr.mozilla.org/comm-central/source/mailnews/local/src/nsPop3Sink.cpp#890, which is the only place we appear to set it, and see if I can make sense of it. If you have time to look at it too, the extra eyes would be much appreciated.

::: comm-release-old/mailnews/local/src/nsPop3Sink.cpp
@@ +312,5 @@
> +  // bug 589870 - need to store actual number of messages downloaded before
> +  // changing record. If mail downloaded control variable will be one
> +  // more than total, so need to check and correct.
> +  uint32_t numDownloadedForStatusMsg;
> +  numDownloadedForStatusMsg = (m_numMsgsDownloaded > 0)?(uint32_t)m_numMsgsDownloaded - 1:0;

Normally I would put the declaration and assignment on the same line, but in this case that would lead to an extra-long line and then we'd have to argue about line folding ;->
Comment on attachment 722205 [details] [diff] [review]
Displays the correct number of Downloads when a filter moves or marks a message

Irving's been looking at this, so I'll let him review it.
Attachment #722205 - Flags: review?(mbanner)
Attachment #722205 - Flags: review?(irving)
Attachment #722205 - Flags: feedback?(irving)
Comment on attachment 722205 [details] [diff] [review]
Displays the correct number of Downloads when a filter moves or marks a message

Review of attachment 722205 [details] [diff] [review]:
-----------------------------------------------------------------

I've done some more digging through the code, and I'm inclined to agree with Eijk in Comment 27 (https://bugzilla.mozilla.org/show_bug.cgi?id=589870#c27) that the problem is more likely in the lines of code around nsPop3Sink.cpp:320 in the current version.

I haven't been able to reproduce the problem myself; it's possible that I'm not configuring filters the same way as those who are having problems.
Attachment #722205 - Flags: review?(irving) → review-
Here is a patch that adds some additional tracing around how we calculate the number of new messages to display while downloading POP.

If someone who can reproduce the bug is willing to try running with this patch, and capture the POP3 log, it would be helpful.

To capture the log, set the following environment variables:

NSPR_LOG_MODULES=pop3:5
NSPR_LOG_FILE=(file name of your choice)

Note that this log file will contain the contents of downloaded messages; you may want to remove any lines that contain the string "RECV:" and review the file to make sure there isn't other private information in the log before you attach it to the bug.

If you'd like to try this patch and you don't have the ability to build your own patched copy of Thunderbird, let me know and I'll create a "try" build for you.
Flags: needinfo?(firefox.bugs)
Attached file Log generated from test patch (obsolete) —
This log was generated from a test patched build launched with the command:
$ export NSPR_LOG_MODULES=pop3:5 && export NSPR_LOG_FILE=/home/*****/Desktop/TB_download.log && mozilla-thunderbird

I hope it provides useful insight.

KJP
Flags: needinfo?(firefox.bugs)
Comment on attachment 734589 [details]
Log generated from test patch

Thanks for the log; I'll have time to look at it tonight or tomorrow.
Attachment #734589 - Flags: feedback?(irving)
Thanks for the log, it points at something very interesting. The new logging shows lines like:

1107511072[7f4d40d53370]: EndMailDelivery adjusting new count: folder 0, m_numNewMessages -1, m_numNewMessagesInFolder 0

Tracing through the places where m_numNewMessages gets set, I suspect it's in the code where we account for new messages being moved by filters; I'm not sure whether the problem is specific to a particular kind of filter. I've added some additional logging to my patch; code I'm suspicious of right now is related to m_newMailParser->m_numNotNewMessages around http://mxr.mozilla.org/comm-central/source/mailnews/local/src/nsPop3Sink.cpp#922. I suspect the problem is in the way we count "not new" messages at http://mxr.mozilla.org/comm-central/source/mailnews/local/src/nsParseMailbox.cpp#2300; because we both subtract the message from the folder's new count and separately count the message as "not new".

If you could run another test with the new patch, you can probably figure out from there where the count is going wrong.
Attachment #732817 - Attachment is obsolete: true
Attachment #735294 - Flags: feedback?(firefox.bugs)
Attachment #734589 - Flags: feedback?(irving)
This is the log from the modified diagnostic patch. I've looked through but can't really see what's going on, but hope the info is clear to anyone who knows TB better.
Attachment #734589 - Attachment is obsolete: true
Hmmm, maybe I'm reading this wrong, but at line 13194 the log appears to report that numNewMessages is set to the number to be downloaded (number the server reports as unread?). Then at 14927 it's still 2, but there are now 3 numNotNewMessages and looking above at 14924 I see m_numMsgsDownloaded as 4.
Not knowing exactly what these variables mean I don't know how they should be calculated, but -1 could either be 4-(2+3) or 2-3.
Or  maybe it's something else completely.
Flags: needinfo?(irving)
What info do you need? I have supplied what you last asked for on 9th April.
Well, I've had no answer to my question. It doesn't look as if anyone's interested any more. Don't think I'll waste my time on it. PCLinuxOS uses my patch and that's good enough for me.

Why should I try to help upstream if no one's listening and they can't even have the courtesy to reply after nearly a month?

I'm clearing the flag so your system will stop nagging me.
Flags: needinfo?(irving)
(In reply to firefox.bugs from comment #63)
> I'm clearing the flag so your system will stop nagging me.

Errm, that's strange, the system shouldn't have been nagging you. The needinfo request was by Irving for nagging himself so that he'd remember to take another look at this. Let me re-set that, and if you get nagged again, then please let me know as that would sound like an issue in bugzilla.
Flags: needinfo?(irving)
Thanks.

I think the problem is that when submitting the log in reply to Irving's last request I either did not see or did not realise I had to tick the box to cancel the flag, so there was still an old flag outstanding. I think this is a different one from the one I cancelled yesterday, but I'll check again after submitting this (which I hope will cancel the right flag).
Flags: needinfo?(irving)
No, that's wrong - it's cancelled the needinfo flag again. The flag I think I need to cancel or answer is the  feedback?(firefox.bugs@instabook.com) at the bottom of comment 59, but I can't find anywhere to do that.
Reset needinfo - I hope!
Flags: needinfo?(irving)
(In reply to firefox.bugs from comment #66)
> No, that's wrong - it's cancelled the needinfo flag again. The flag I think
> I need to cancel or answer is the  feedback?(firefox.bugs@instabook.com) at
> the bottom of comment 59, but I can't find anywhere to do that.

click on the detailsbutton next to the attachment.
Comment on attachment 735294 [details] [diff] [review]
Additional pop3 logging to trace new message counts - v2

Will commenting here enable me to clear the flag?
Attachment #735294 - Flags: feedback?(firefox.bugs)
Thanks - I'd been in that details section several times but couldn't see anything other than a line stating the flag was present with no obvious way to edit it. Thanks for cancelling it.
It looks very much like this occurs when messages are downloaded, then all of them are sent to a specific folder with a filter. In my case the filter also uses the Do Not Notify option from some plug-in I've downloaded.

Looking at the Activity manager I see "No messages downloaded" or some specific number of messages from time to time, but long strings of:
4294967295 messages downloaded
intermixed with
4294967294 messages downloaded

But I've also seen:
4294967291 messages downloaded
4294967292 messages downloaded
4294967293 messages downloaded

It would appear number reported depends on the number of messages filtered out. I often get one or two SPAM messages with each download, but sometimes more, and they are filtered to Trash. So my guess is 4294967295 means one messages was filtered out, and each number lower corresponds to more messages. When some messages are not filtered to trash it appears that the real number of messages downloaded is reported (but it is possible that the number reported is reduced by the number filtered out - I've not checked that to be sure).
I spent a bunch of time looking into this, but was unable to completely sort out the problem. Unread message counting is a mess; as comment 72 shows, there are places where we underflow unsigned integers, and there are other places where we add and remove adjustments to the unread count multiple times in ways that are almost certainly incorrect.

I'm also frustrated by the incorrect unread counts that Thunderbird displays, but unfortunately I don't have time to spend on this bug. I'd be happy to help anyone who wants to untangle the current mess.
Flags: needinfo?(irving)
Still happening as of May 2014 in TB 24.5.0: I got "4294967295 messaged downloaded".
4294967295 = 0xFFFFFFFF, so clearly an unsigned int counted down from 0 to -1, as comment #73 states: "underflow of unsigned integer". Last seen in June 2015 with TB 40 (or 38?).
All this is **** ridiculous.
Five years with such a bug because you're trying to deduce the number of downloaded message from the number of unread messages plus those that were marked as read and stuff like that?? No surprise that you get the number wrong at some point.

WTF!? Why don't just display the number of messages that were actually downloaded? That shouldn't need any arithmetics at all!

I bet some day the underflow will be fixed, but the computations will remain messy and buggy, but nobody will notice because the numbers will be believable.
I disagree, teo8976.  People have been layering on "fixes" for more than seven years now, and people still notice that the numbers are wrong.  I'm not sure why you think profanity will motivate people to _actually_ fix the problem instead of just adding yet another "+1" or "-1", but to each their own, right?  :)
I suppose we need a clear definition of what we mean by messages downloaded before anyone can even start to offer a fix. Is a message whose headers only are downloaded, downloaded? Is a message deleted from the server but not downloaded to be counted? Is a message downloaded to trash still to be counted?

In other words, what are you trying to count? What do you want to display? How useful is it anyway, given it's been wrong for many years. Maybe just "No messages to download" or "Messages downloaded" without a number is enough!
> I suppose we need a clear definition of what we mean by messages downloaded

I guess you're talking about IMAP.
For POP3 it's pretty obvious: the number of messages physically downloaded, no matter what happens to them after download.
You evidently don't understand POP3, then. It is still possible to set a filter to reject a message on the basis of the headers, so does downloading the headers count as downloading the message? Does downloading the message to a trash folder count?

Opinions will probably differ.

All I'm suggesting is that if the specification isn't clear it's not surprising if solutions are confusing.
> You evidently don't understand POP3, then. It is still possible to
> set a filter to reject a message on the basis of the headers,

You're right, I didn't know that.

> so does downloading the headers count as downloading the message?
Obviously no.

> Does downloading the message to a trash folder count?
Obviously yes.


> Opinions will probably differ.

I give you that. No matter how obvious something is, there will almost surely be someone who will have a different opinion

> All I'm suggesting is that if the specification isn't clear 
> it's not surprising if solutions are confusing.

I completely agree with you that the specification is to be made clear before anything is done.
Regarding this particular issue, I don't think there are confusing solutions to an unclear specification: by what I get from a few comments (I haven't inspected the actual source code) there seems to be more of an accumulation of bad hacks and patch-arounds (yielding bugs), even in the parts where there's no confusion about what is intended.
Component: Message Reader UI → Mail Window Front End
Summary: Too large number displayed after automatic e-mail download → Too large number for downloaded messages displayed in status bar after automatic e-mail download
I encountered this again.
No filtered into trash.
No spam.
Attached image Screenshot
message count of 0xFFFFFFFE at Activity Manager is observed in Bug 1205525.
  4294967294 == 0xFFFFFFFE == 32bits unsigned integer interpretation of -2 of 32bits signed integer
Setting dependency for ease of problem analysis and tracking.
Adding 0xFFFFFFFE to bug summary for ease of understanding problem and search.
Depends on: 1205525
Summary: Too large number for downloaded messages displayed in status bar after automatic e-mail download → Too large number for downloaded messages displayed in status bar after automatic e-mail download, for example, 4294967294==0xFFFFFFFE==-2 of 32bits signed integer
Bug 1205525 is for phenomenon of "message filter move upon download silently fails, if moving messages to filter move target folder is in progress". So, a suspect is "filter move failure".

I expected this bug to be fixed by now. But with TB 60.7.1 (64-bit) (MacOS) I observed it again.

Attached image The bug still lives... (obsolete) —
Attachment #9087299 - Attachment is obsolete: true
Attached image happened today.

The number as shown is definitely incorrect.

Comment on attachment 9087299 [details]
The bug still lives...

I should not list e-mail content in the screen dump. Sorry.

Adding an attachment to show bug occurring in Thunderbird 68.1.2 (32-bits) - Windows 10

Depends on: 168648
OS: Windows 7 → All
Severity: minor → S4
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: