Closed Bug 673703 Opened 13 years ago Closed 7 months ago

Thunderbird is limited to 10000 temp files, stops sending emails if nsmail-9999.tmp exists, and fails to open attachment if <attachment_filename>-9999.attachment_extension exists

Categories

(Thunderbird :: General, defect)

defect

Tracking

(thunderbird_esr115? fixed, thunderbird118 fixed)

RESOLVED FIXED
119 Branch
Tracking Status
thunderbird_esr115 ? fixed
thunderbird118 --- fixed

People

(Reporter: demon, Assigned: welpy-cw)

References

(Blocks 1 open bug)

Details

(Keywords: ux-error-prevention, Whiteboard: [has pointers to code])

Attachments

(1 file, 2 obsolete files)

User Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.16) Gecko/20110701 Iceweasel/3.5.16 (like Firefox/3.5.16)
Build ID: 20110701113851

Steps to reproduce:

Regarding all versions at least at Win XP.

Sending email fails if the limit of 10000 temp files is reached. I.e. there exists nsmail-9999.tmp in C:\Documents and Settings\User\Local Settings\Temp.

The error message is -- if email contains included files, then that the user has not enough rights to this files or if there are no files included then that the network preferences are wrong.


Actual results:

In fact TB is limited to 10000 temp files and cannot create a new temp file beyond nsmail-9999.tmp. TB fails without appropriate error message. It also does not remove its temp files, at it should.


Expected results:

TB shall remove its temp-files on its own and shall not be limited to 10000 temp-files.
Workaround: remove all ns*.tmp in C:\Documents and Settings\User\Local Settings\Temp
problem place #1: mozilla/xpcom/io/nsLocalFileCommon.cpp: line 172
mktemp shall be considered as replacement
I can confirm it for linux and tb 5.0

Steps to reproduce:
$ cd /tmp
$ touch nscopy.tmp; touch nsemail.eml
$ for i in `seq 1 9999`; do touch nscopy-$i.tmp; touch nsemail-$i.eml; done
Then try to write new message and save it as draft
duplicate of bug 389132?
(In reply to Wayne Mery (:wsmwk) from comment #4)
> duplicate of bug 389132?

Not really - this one would block.(In reply to Dimitri Sokolyuk from comment #2)

> problem place #1: mozilla/xpcom/io/nsLocalFileCommon.cpp: line 172
> mktemp shall be considered as replacement

Dimitri do you think you could come up with a patch to fix the issue ?
Component: General → XPCOM
Product: Thunderbird → Core
QA Contact: general → xpcom
Version: 5.0 → Trunk
(In reply to Ludovic Hirlimann [:Usul] from comment #5)
> Dimitri do you think you could come up with a patch to fix the issue ?

Right now I'm a little out of time, but I'll see what I can do. However it's only a part of a problem. Much bigger problem is, that TB misses somewhere to destroy it's temporary files. I haven't find a location of this second problem right now.
(In reply to Wayne Mery (:wsmwk) from comment #7)
> NB bug 377621 is one potential source of these files. And it has a
> unfinished, uncommitted patch.

I don't think the nsmail.tmp files come from that, since that bug is about adding a new set of temporary files for drag-and-drop on GTK. Currently, temp files aren't created at all with drag-and-drop on GTK.
xref bug 389132, for ease of tracking & search.

As I wrote in bug 389132 comment #15, an easiest/simplist way to generate undeleted nsmail-NNNN.tmp is;
  Compose a mail and attach many mails, and "Send Later".
  Cancel mail sending(or Kill Tb), while creating many nsmail-NNNN.tmp.
As seen in bug 389132 comment #11, Tb tries to find free NNNN for nsmail-NNNN.tmp from nsmail.tmp/nsmail-1.tmp always. I was interested in "creation fail when NNNN==9999" case in that bug, but I didn't check it.

(In reply to Dimitri Sokolyuk from comment #0)
> It also does not remove its temp files, at it should.

(A) For "cancel of mail sending" case: Tb should remove temp files.
(B) For crash of Tb case:
    Application usually can not request remove if crash happens.
    So, user is usually responsible to remove garbages in /tmp or \Temp,
    or to clean up /tmp or \Temp periodically.
    It's ordinal design of OS used by PC.
    "Clean up by user" will be mandatory, even if system call like "mktemp"
    will be used, unless OS will be responsible to remove remained temp file
    after crash of an application.
Blocks: 389132
This bug is applicavle to temp file to open appachment by application.

For example, open attachment of filename=Voice.PDF by Adobe Reader on MS Win.
(1) Tb uses random name like "0QuEzw0F" (no quote) for temp file.
(2) Tb deletes C:\DOCUME~1\wada\LOCALS~1\Temp\0QuEzw0F.TXT (without ".part"). 
(3) Tb creates C:\DOCUME~1\wada\LOCALS~1\Temp\0QuEzw0F.TXT.part (with ".part"),
    and write attachment data to this file.
(4) Tb tries to rename C:\DOCUME~1\wada\LOCALS~1\Temp\0QuEzw0F.TXT.part
    to Voice.PDF.
    If Voice.PDF already exists, try to rename to Voice-1.PDF.
    If Voice-{N}.PDF already exists, try to rename to Voice-{N+1}.PDF,
    where N == 1 to 9998 (Max {N+1} == 9999).
(5-A) If rename to Voice.PDF or Voice-X.PDF (X == 1 to 9999) is successful,
    C:\DOCUME~1\wada\LOCALS~1\Temp\Voice-X.PDF is passed to application.
(5-B) If rename to Voice-9999.PDF fails, following dialog is shown, and "open attachment by application" fails.
> Downloading C:\DOCUME~1\wada\LOCALS~1\Temp\0QuEzw0F.TXT.part
>   ! C:\DOCUME~1\wada\LOCALS~1\Temp\0QuEzw0F.TXT.part could not be saved,
>        because an unknown error occurred.
>     Try saving to a different location.
>              [ OK ]

Problems in above.
(a) File name which is deleted at step (2) (without .part)  is different from file name which is used for data writing at step (3) (with .part).
(b) Because Tb tries Voice-1.PDF to Voice-9999.PDF serially, it usualy takes long.
(c) Error message at step (5-B) is incorrect, because writing data to 0QuEzw0F.TXT.part itself is successful. Error message should indicate file name of Voice-9999.PDF in the dialog.

Note-1:
If "Cancel" is requested at "Open Dialog", temp file of C:\DOCUME~1\wada\LOCALS~1\Temp\0QuEzw0F.TXT.part is normally deleted by Tb 8. So, garbage is not produced by "Cancel" in this case. 

Note-2:
If "open attachment by application" is requested multiple times for same attachment at same time by user, Tb writes data to differnt {random_string}.PDF, and renames to different Voice-NNNN.PDF.

Note-3:
If rename to Voice-A.PDF, Voice-B.PDF, Voice-C.PDF, ..., is successful, these temp files are used by external application. So, these files are not deleted by Tb while Tb is running.
When browser.download.manager.retention=1(Tb's default=1), these temp files are deleted upon termination of Tb, if and only if termination of Tb is successful and delete request by Tb is successful.
i.e.
If system crash, power failure, Tb's hung/freeze etc. occurs before deletion of temp files by Tb, or if temp file is still opened by application when Tb tries to delete the temp file upon termiation of Tb, temp file remains in Temp directory of system.
OS usually never deletes such remaining temp files automatically. So, user needs to delete such garbage files by himself.  It's OS's design.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Summary: TB is limited to 10000 temp files and stops sending emails if nsmail-9999.tmp exists. → TB is limited to 10000 temp files, and stops sending emails if nsmail-9999.tmp exists, and fails to open attachment by application if <attchment_filename>-9999.attachment_extention exists.
Impressive that this is still broken almost two years after the bug was filed.
Whiteboard: [has pointers to code]
Jonathan, do you still have 9999 nsmail temp files existing?
I think recently there were some fixes to not leave stray temp files around.

Does anybody think we should add some code to TB to delete all stray temp files belonging to TB on startup (e.g. nsmail-*.tmp, but there still may be a clash if Seamonkey is running alongside) ?
(In reply to :aceman from comment #18)
> Jonathan, do you still have 9999 nsmail temp files existing?
> I think recently there were some fixes to not leave stray temp files around.
> 
> Does anybody think we should add some code to TB to delete all stray temp
> files belonging to TB on startup (e.g. nsmail-*.tmp, but there still may be
> a clash if Seamonkey is running alongside) ?

Something needs to be done bug I'm ambivalent about a wallpaper solution that hides symptoms of real problems/incorrect code.  In other words, I'm sure there are cases where we can outright fix problems.  https://bugzilla.mozilla.org/buglist.cgi?f1=OP&o3=anywordssubstr&list_id=7707974&short_desc=temp%20tmp%20temporary%20nseml%20nstmp%20nstemp&resolution=---&resolution=FIXED&resolution=EXPIRED&classification=Client%20Software&classification=Components&o2=nowordssubstr&f4=CP&query_format=advanced&j1=OR&f3=keywords&f2=short_desc&short_desc_type=anywords&v2=templat%20biff%20contact%20cert&product=MailNews%20Core&product=Thunderbird lays out the history and what's left to fix.

bug 553165 is the wallpaper idea.

(In reply to :aceman from comment #18)

Jonathan, do you still have 9999 nsmail temp files existing?
I think recently there were some fixes to not leave stray temp files around.

I can still reproduce sticking "nsqmail-*.tmp" files by placing mails in the Send-Later folder and then calling Send Unsend Messages while the server is down.
https://searchfox.org/comm-central/rev/68ba4decd0f0b2e4081c2be892fc12a5e8f44784/mailnews/compose/src/nsMsgSendLater.cpp#557

I just had this issue in 2021, "Assembling message" then "sending of the message failed" suddenly. After reading this thread, my nsemail* files were 9999 and nsmail was in the 3000s. Deleted them all, problem solved. Thanks and yes please write proper error message when this happens. Much appreciated!

Summary: TB is limited to 10000 temp files, and stops sending emails if nsmail-9999.tmp exists, and fails to open attachment by application if <attchment_filename>-9999.attachment_extention exists. → thunderbird is limited to 10000 temp files, and stops sending emails if nsmail-9999.tmp exists, and fails to open attachment by application if <attchment_filename>-9999.attachment_extention exists.
Severity: normal → S3

The severity field for this bug is relatively low, S3. However, the bug has 4 duplicates.
:nika, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Flags: needinfo?(nika)

The 10,000 temp file limit still exists. It wouldn't be that hard to increase it, but if there's some kind of temp file leak in Thunderbird that's only going to delay the issue for people.

I can bump the limit to 99,999 and maybe that will help...

Assignee: nobody → continuation

Apparently Thunderbird hits the current limit. Maybe increasing by a factor of 10 will help.
I'm not increasing it further so I don't have to worry about file length issues.

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.

Flags: needinfo?(nika)

From a quick scan I don't see code in Thunderbird which handles deleting the temporary nsqmail.temp files created here: https://searchfox.org/comm-central/rev/ddb726dfb927d29debb26fb4b7cb8508b0c38c97/mailnews/compose/src/nsMsgSendLater.cpp#556, which might be a cause of the temporary file build-up (at least for nsqmail*.temp). I expect there are other things like that in thunderbird for the other types of temporary files which are leaking.

We might still do something on the xpcom side to allow large numbers of temporary files, but this bug specifically seems like it is probably thunderbird-specific.

Component: XPCOM → General
Product: Core → Thunderbird
Attachment #9298855 - Attachment is obsolete: true
Summary: thunderbird is limited to 10000 temp files, and stops sending emails if nsmail-9999.tmp exists, and fails to open attachment by application if <attchment_filename>-9999.attachment_extention exists. → Thunderbird is limited to 10000 temp files, stops sending emails if nsmail-9999.tmp exists, and fails to open attachment if <attachment_filename>-9999.attachment_extension exists

I found code to clean up some temporary files when Thunderbird starts up.
https://searchfox.org/comm-central/source/mailnews/compose/src/nsMsgComposeService.cpp#121

nsresult nsMsgComposeService::Init() {
  nsresult rv = NS_OK;

  Reset();

  AddGlobalHtmlDomains();
  // Since the compose service should only be initialized once, we can
  // be pretty sure there aren't any existing compose windows open.
  MsgCleanupTempFiles("nsmail", "tmp");
  MsgCleanupTempFiles("nsemail", "html");
  MsgCleanupTempFiles("nscopy", "tmp");
  return rv;
}

I think the following code should be added.

  MsgCleanupTempFiles("nsemail", "eml");
  MsgCleanupTempFiles("nsqmail", "tmp");

nsmail-xxxx.tmp is cleaned up on Thunderbird startup. (since Thunderbird 16)
It is nsemail-xxxx.eml that is causing the sending to stop with NS_ERROR_FILE_TOO_BIG error, and I don't think it is appropriate to include nsmail-9999.tmp in this bug summary.

I checked the behavior of file removing by MsgCleanupTempFiles().
I created the following files in the temp folder and then started Thunderbird.
nscopy.tmp, nscopy-1.tmp, nscopy-2.tmp, ... , nscopy-9999.tmp

In Windows environment, it can be executed with the following PowerShell script.

# Testing MsgCleanupTempFiles()

New-Item $env:temp\nscopy.tmp -type file -Force
for ($i=1; $i -le 9999; $i++) {
  $f = $env:temp + "\nscopy-" + $i + ".tmp"
  New-Item $f -type file -Force
}
Start-Process -FilePath "C:\Program Files\Mozilla Thunderbird\thunderbird.exe"

Actual results:
Removed nscopy.tmp, nscopy-1.tmp, nscopy-2.tmp, ... , nscopy-9998.tmp, but nscopy-9999.tmp remains.

NS_ERROR_FILE_TOO_BIG error occurs when the index reaches 9999, so files up to 9999 should be removed.
I think the code of MsgCleanupTempFiles() should be modified as below.
https://searchfox.org/comm-central/source/mailnews/base/src/nsMsgUtils.cpp#1271

  } while (exists && ++index < 10000);

  } while (exists && index++ < 10000);
Flags: needinfo?(continuation)

I don't actually work on Thunderbird, so I think somebody else would be in a better position to fix this. I forgot to clear myself from the assigned field when this got moved out of XPCOM.

Assignee: continuation → nobody
Flags: needinfo?(continuation)

The mechanism by which the nsemail-xxxx.eml file is increased is unknown, but I have seen reports of NS_ERROR_FILE_TOO_BIG errors that result in emails not being sent.
MozillaZine.jp forum (Japanese)

The NS_ERROR_FILE_TOO_BIG error can be prevented by automatically deleting the temporary file when Thunderbird starts.

I would appreciate it if someone could fix this bug.

Flags: needinfo?(mkmelin+mozilla)

I submitted a job with a patch that incorporate the proposed changes.
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=4166f87076876f916196758da9b6a60b636dc982

The patch:
https://hg.mozilla.org/try-comm-central/rev/f626bf44149071da679eb890b77743c0e7b87437
I feel a bit awkward when there are still so many OTHER errors to pronounce the health of the patch although I did not see anything that stood as new bugs.

ISHIKAWA, please submit the patch through phabricator. I don't see any new problems.

Flags: needinfo?(mkmelin+mozilla)

https://hg.mozilla.org/try-comm-central/rev/f626bf44149071da679eb890b77743c0e7b87437#l2.9
You've added

+  MsgCleanupTempFiles("nsemail", "eml");
+  MsgCleanupTempFiles("nsqmail", "tmp");

This is correct, since both temp file names are used (as already pointed out in comment #29):
https://searchfox.org/comm-central/search?q=nsemail.*eml&path=&case=false&regexp=true
https://searchfox.org/comm-central/search?q=nsqmail.*tmp&path=&case=false&regexp=true

Can you please also add nsemail.tmp
https://searchfox.org/comm-central/search?q=nsemail.*tmp&path=&case=false&regexp=true
and remove nsemail.html which is not used any more:
https://searchfox.org/comm-central/search?q=nsemail.*html&path=&case=false&regexp=true
See also comment in test_temporaryFilesRemoved.js. If you remove that, test_staleTemporaryFileCleanup.js needs to be adjusted accordingly.

We suggest to credit EarlgreyTea <earlgreypicard@gmail.com> with the analysis here.

(In reply to Magnus Melin [:mkmelin] from comment #36)

ISHIKAWA, please submit the patch through phabricator. I don't see any new problems.

Will do hopefully this week. I have not set up phabricator completely. I tried once several years ago, but it was too early . I mean the internal usage documentation was sparse and so I could not get it to work meaningfully.

(In reply to betterbird.project+6 from comment #37)

https://hg.mozilla.org/try-comm-central/rev/f626bf44149071da679eb890b77743c0e7b87437#l2.9
You've added

+  MsgCleanupTempFiles("nsemail", "eml");
+  MsgCleanupTempFiles("nsqmail", "tmp");

This is correct, since both temp file names are used (as already pointed out in comment #29):
https://searchfox.org/comm-central/search?q=nsemail.*eml&path=&case=false&regexp=true
https://searchfox.org/comm-central/search?q=nsqmail.*tmp&path=&case=false&regexp=true

Can you please also add nsemail.tmp
https://searchfox.org/comm-central/search?q=nsemail.*tmp&path=&case=false&regexp=true
and remove nsemail.html which is not used any more:
https://searchfox.org/comm-central/search?q=nsemail.*html&path=&case=false&regexp=true
See also comment in test_temporaryFilesRemoved.js. If you remove that, test_staleTemporaryFileCleanup.js needs to be adjusted accordingly.

We suggest to credit EarlgreyTea <earlgreypicard@gmail.com> with the analysis here.

Let me digest this and incorporate necessary changes. Please wait for a few days.

Let me digest this and incorporate necessary changes.

You can see it in the updated version.

I think.
The patch to fix this bug has already been shown.
However, I do not have a build environment and no way to commit the patch.
Ishikawa-san also needs time to get phabricator set up and working.
If someone who can already use phabricator can submit the patch, we can move forward to the next step.

Attached file Bug 673703 - remove temporary files (obsolete) —

(In reply to EarlgreyTea from comment #40)

I think.
The patch to fix this bug has already been shown.
However, I do not have a build environment and no way to commit the patch.
Ishikawa-san also needs time to get phabricator set up and working.
If someone who can already use phabricator can submit the patch, we can move forward to the next step.

Copmment 41
My first attempt to use moz-phab seemed successful, except for the missing reviewer.

Someone can comment on the review, I think.

I am afraid I am a follower of the school of thought that favors independent tools rather than
intewound tool like moz-phab that is tied very deep with MERCURIAL, but that is because I have used HG MQ extension and git and like the ease of pushing and popping patches locally quite often.
Sure, once you are deep into the workflow of HG and other cloud services, I believe moz-phab may help a lot.
Except, I still can't figure out how to handle almost two dozen different patch sets that handle two dozen different bugs/issues neatly
once in my local queue without HG MQ.
Anyway, I will be learning a bit more about HG EVOLUTION, I suppose.
(It would be great if there is a testbed for moz-phab. I almost screwed up by posting many locally brewing patches without my intention.)

Chiaki, is the any issue following our advice in comment #37? See comment #39 for a full version of the patch.

The file types that should be added are nsemail.eml, nsemail.tmp and nsqmail.tmp. nsemail.html should be removed and the tests adjusted accordingly.

Flags: needinfo?(ishikawa)

(In reply to betterbird.project+8 from comment #43)

Chiaki, is the any issue following our advice in comment #37? See comment #39 for a full version of the patch.

The file types that should be added are nsemail.eml, nsemail.tmp and nsqmail.tmp. nsemail.html should be removed and the tests adjusted accordingly.

That is a simple oversight. I thought I had modified the local patch accordingly, but obviously I forgot to do that.
Now, I have to learn the process of updating a patch submitted via phabricator. Please wait for a day or two.

Flags: needinfo?(ishikawa)

To update a patch submitted with phab, just hg up <rev>, do the changes, hg amend and then moz-phab .
As long as the "Differential Revision: https:.... " line remains inside the commit, moz-phab will know it's the same, and send an update instead

(In reply to ISHIKAWA, Chiaki from comment #44)

Please wait for a day or two.

It seems it was two weeks, not two days. Or maybe two months.

Sorry my day job and other stuff keeps me busy. I wish there were 40 hours in a day.

Assignee: nobody → h.w.forms
Status: NEW → ASSIGNED

(In reply to EarlgreyTea from comment #40)

The patch to fix this bug has already been shown.
However, I do not have a build environment and no way to commit the patch.
Ishikawa-san also needs time to get phabricator set up and working.
If someone who can already use phabricator can submit the patch, we can move forward to the next step.

Hope, that's ok, ISHIKAWA.

Attachment #9352213 - Attachment description: Bug 673703 - Fix non-deletion of nsmail-9999.tmp. Analysis by EarlgreyTea <earlgreypicard@gmail.com>. r=mkmelin → Bug 673703 - Fix non-deletion of nsmail-9999.tmp. r=mkmelin
Target Milestone: --- → 119 Branch

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/57adabcbe8c7
Fix non-deletion of nsmail-9999.tmp. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED

(In reply to Hartmut Welpmann [:welpy-cw] from comment #49)

(In reply to EarlgreyTea from comment #40)

The patch to fix this bug has already been shown.
However, I do not have a build environment and no way to commit the patch.
Ishikawa-san also needs time to get phabricator set up and working.
If someone who can already use phabricator can submit the patch, we can move forward to the next step.

Hope, that's ok, ISHIKAWA.

Thank you. That is fine.
I have been down with Covid-19 and recover has been very slow.
Thank you for taking care of the pending patch.

(In reply to ISHIKAWA, Chiaki from comment #51)

I have been down with Covid-19 and recover has been very slow.
Thank you for taking care of the pending patch.

My pleasure. You could mark your revision as abandoned ("Add Action..." down below in Phabricator).
I wish you all the best for your further recovery!

Comment on attachment 9346419 [details]
Bug 673703 - remove temporary files

[Triage Comment]
Approved for beta

Attachment #9346419 - Flags: approval-comm-beta+

Comment on attachment 9352213 [details]
Bug 673703 - Fix non-deletion of nsmail-9999.tmp. r=mkmelin

[Triage Comment]
Approved for beta

Attachment #9352213 - Flags: approval-comm-beta+
Attachment #9346419 - Attachment is obsolete: true

Comment on attachment 9346419 [details]
Bug 673703 - remove temporary files

[Triage Comment]
welpy-cw, Thanks for the correction. I missed that only one had been checked in.

Attachment #9346419 - Flags: approval-comm-beta+ → approval-comm-beta-
Attachment #9352213 - Flags: approval-comm-esr115?

Comment on attachment 9352213 [details]
Bug 673703 - Fix non-deletion of nsmail-9999.tmp. r=mkmelin

[Triage Comment]
Approved for esr115

Attachment #9352213 - Flags: approval-comm-esr115? → approval-comm-esr115+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: