Bug 1595343 Comment 0 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

I noticed strange errors reported by the following ASSERTION
during the run of |make mozmill|  of C-C TB (FULL DEBUG version).

###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/mailnews/compose/src/nsMsgAttachment.cpp, line 213

Strange still is that they are sporadic.

The bug appeared sporadically in |make mozmill|.  Number of
appearances, one or two, differs from time time.

On my local log, it started to appear in February 2016.

- Feb 2016

/FF-NEW/log780-mozmill.txt.gz
[29790] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NREF-COMM-CENTRAL/comm-central/mailnews/compose/src/nsMsgAttachment.cpp, line 244
[29790] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NREF-COMM-CENTRAL/comm-central/mailnews/compose/src/nsMsgAttachment.cpp, line 244

/FF-NEW/log782-mozmill.txt.gz
[27271] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NREF-COMM-CENTRAL/comm-central/mailnews/compose/src/nsMsgAttachment.cpp, line 244
[27271] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NREF-COMM-CENTRAL/comm-central/mailnews/compose/src/nsMsgAttachment.cpp, line 244
/FF-NEW/log783-xpcshell.txt.gz

- July 2017
/FF-NEW/log960-mozmill.txt:27493:[6655] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NREF-COMM-CENTRAL/comm-central/mailnews/compose/src/nsMsgAttachment.cpp, line 244
/FF-NEW/log960-mozmill.txt:27538:[6655] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NREF-COMM-CENTRAL/comm-central/mailnews/compose/src/nsMsgAttachment.cpp, line 244

/FF-NEW/log967-mozmill.txt:27712:[17157] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NREF-COMM-CENTRAL/comm-central/mailnews/compose/src/nsMsgAttachment.cpp, line 244


August 2019
/FF-NEW/log1055-mozmill-fixed-stack.txt:7702:[9326, Main Thread] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NREF-COMM-CENTRAL/mozilla/comm/mailnews/compose/src/nsMsgAttachment.cpp, line 214

.... some omissions from the summer to now.

Nov 2019
/FF-NEW/log1115-mozmill-fixedstack.txt:8186:[16433, Main Thread] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/mailnews/compose/src/nsMsgAttachment.cpp, line 213
/FF-NEW/log1115-mozmill-fixedstack.txt:8229:[16433, Main Thread] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/mailnews/compose/src/nsMsgAttachment.cpp, line 213


The bug comes from the following test:

TEST-START | /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/mail/test/mozmill/attachment/test-attachment-size.js | test_attached_message_with_attachment


I checked the code. The message is printed by NS_ERROR() in Full
Debug build of C-C TB.

mozilla/comm/mailnews/compose/src/nsMsgAttachment.cpp


```
nsresult nsMsgAttachment::DeleteAttachment() {
  nsresult rv;
  bool isAFile = false;

  nsCOMPtr<nsIFile> urlFile;
  rv = NS_GetFileFromURLSpec(mUrl, getter_AddRefs(urlFile));
  NS_ASSERTION(NS_SUCCEEDED(rv), "Can't nsIFile from URL string"); <== THIS
  if (NS_SUCCEEDED(rv)) {
    bool bExists = false;
    rv = urlFile->Exists(&bExists);
    NS_ASSERTION(NS_SUCCEEDED(rv), "Exists() call failed!");
    if (NS_SUCCEEDED(rv) && bExists) {
      rv = urlFile->IsFile(&isAFile);
      NS_ASSERTION(NS_SUCCEEDED(rv), "IsFile() call failed!");
    }
  }

  // remove it if it's a valid file
  if (isAFile) rv = urlFile->Remove(false);

  return rv;
}
```

I found that there is a problem here.  The assertion is triggered by
the failure of a call to |NS_GetFileFromURLSpec()|. And when that call
fails, the removal of the attachment file does not happen.
|isAFile| is initialized to |false| and does not change the value when 
the call to |NS_GetFileFromURLSpec()| fails. Thus file |Remove()| call
never takes place.


In any case, this is possibly a serious bug.
We do not want to leave
an attachment hanging around in temporary directory randomly (even if
the permission is set to user RW only).

The user may have thought that the particular sensitive attachment has
been sent and s/he may have deleted the original file and even from
the attachment of a e-mail copy in Sent folder to be sure.  And yet
the copy may be left in the temporary directory from time to time. A
surprise, isn't it?

One possible reason for sporadic nature of this issue.:

Looking at the stack trace from NS_ERROR(), I see there is a gabarge
collection in the stacktrace before some Release calls are made to
release objects. Hmm...
So maybe this is one last call to GC before shutdown?

It seems that this DeleteAttachment is called by the releasing of an
object (Attachment?) due to the garbage collection and by the time
this release takes place, other internal infrastructure for the
successful execution of |NS_GetFileFromURLSpec()| may have disappeared
already.  (Now come to think of it, depending on the runs of GC's
traversal when it is invoked and how long, the releasing order of
related object may be semi-random, for that matter.)

The developer may have thought the orphaned object will be relclaimed
eventually by GC, and destruction of object would take care of the
removal of an external file in temporary directory.  However, as we
see here, the temporary file for creating attachment (presumably) is
left around sometimes.
Bad...
I noticed strange errors reported by the following ASSERTION
during the run of |make mozmill|  of C-C TB (FULL DEBUG version).

###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/mailnews/compose/src/nsMsgAttachment.cpp, line 213

Strange still is that they are sporadic.

The bug appeared sporadically in |make mozmill|.  Number of
appearances, one or two, differs from time time.

On my local log, it started to appear in February 2016.

- Feb 2016

/FF-NEW/log780-mozmill.txt.gz
[29790] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NREF-COMM-CENTRAL/comm-central/mailnews/compose/src/nsMsgAttachment.cpp, line 244
[29790] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NREF-COMM-CENTRAL/comm-central/mailnews/compose/src/nsMsgAttachment.cpp, line 244

/FF-NEW/log782-mozmill.txt.gz
[27271] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NREF-COMM-CENTRAL/comm-central/mailnews/compose/src/nsMsgAttachment.cpp, line 244
[27271] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NREF-COMM-CENTRAL/comm-central/mailnews/compose/src/nsMsgAttachment.cpp, line 244
/FF-NEW/log783-xpcshell.txt.gz

- July 2017
/FF-NEW/log960-mozmill.txt:27493:[6655] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NREF-COMM-CENTRAL/comm-central/mailnews/compose/src/nsMsgAttachment.cpp, line 244
/FF-NEW/log960-mozmill.txt:27538:[6655] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NREF-COMM-CENTRAL/comm-central/mailnews/compose/src/nsMsgAttachment.cpp, line 244

/FF-NEW/log967-mozmill.txt:27712:[17157] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NREF-COMM-CENTRAL/comm-central/mailnews/compose/src/nsMsgAttachment.cpp, line 244


August 2019
/FF-NEW/log1055-mozmill-fixed-stack.txt:7702:[9326, Main Thread] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NREF-COMM-CENTRAL/mozilla/comm/mailnews/compose/src/nsMsgAttachment.cpp, line 214

.... some omissions from the summer to now.

Nov 2019
/FF-NEW/log1115-mozmill-fixedstack.txt:8186:[16433, Main Thread] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/mailnews/compose/src/nsMsgAttachment.cpp, line 213
/FF-NEW/log1115-mozmill-fixedstack.txt:8229:[16433, Main Thread] ###!!! ASSERTION: Can't nsIFile from URL string: 'NS_SUCCEEDED(rv)', file /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/mailnews/compose/src/nsMsgAttachment.cpp, line 213


The bug comes from the following test:

TEST-START | /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/mail/test/mozmill/attachment/test-attachment-size.js | test_attached_message_with_attachment


I checked the code. The message is printed by NS_ERROR() in Full
Debug build of C-C TB.

mozilla/comm/mailnews/compose/src/nsMsgAttachment.cpp


```
nsresult nsMsgAttachment::DeleteAttachment() {
  nsresult rv;
  bool isAFile = false;

  nsCOMPtr<nsIFile> urlFile;
  rv = NS_GetFileFromURLSpec(mUrl, getter_AddRefs(urlFile));
  NS_ASSERTION(NS_SUCCEEDED(rv), "Can't nsIFile from URL string"); <== THIS
  if (NS_SUCCEEDED(rv)) {
    bool bExists = false;
    rv = urlFile->Exists(&bExists);
    NS_ASSERTION(NS_SUCCEEDED(rv), "Exists() call failed!");
    if (NS_SUCCEEDED(rv) && bExists) {
      rv = urlFile->IsFile(&isAFile);
      NS_ASSERTION(NS_SUCCEEDED(rv), "IsFile() call failed!");
    }
  }

  // remove it if it's a valid file
  if (isAFile) rv = urlFile->Remove(false);

  return rv;
}
```

I found that there is a problem here.  The assertion is triggered by
the failure of a call to |NS_GetFileFromURLSpec()|. And when that call
fails, the removal of the attachment file does not happen.
|isAFile| is initialized to |false| and does not change the value when 
the call to |NS_GetFileFromURLSpec()| fails. Thus file |Remove()| call
never takes place.


In any case, this is possibly a serious bug.
We do not want to leave
an attachment hanging around in temporary directory randomly (even if
the permission is set to user RW only).

The user may have thought that the particular sensitive attachment has
been sent and s/he may have deleted the original file and even from
the attachment of a e-mail copy in Sent folder to be sure.  And yet
the copy may be left in the temporary directory from time to time. A
surprise, isn't it?

One possible reason for sporadic nature of this issue.:

Looking at the stack trace from NS_ERROR() [I will attach the log with additional dump], I see there is a gabarge
collection in the stacktrace before some Release calls are made to
release objects. Hmm...
So maybe this is one last call to GC before shutdown?

It seems that this DeleteAttachment is called by the releasing of an
object (Attachment?) due to the garbage collection and by the time
this release takes place, other internal infrastructure for the
successful execution of |NS_GetFileFromURLSpec()| may have disappeared
already.  (Now come to think of it, depending on the runs of GC's
traversal when it is invoked and how long, the releasing order of
related object may be semi-random, for that matter.)

The developer may have thought the orphaned object will be reclaimed
eventually by GC, and destruction of object would take care of the
removal of an external file in temporary directory.  However, as we
see here, the temporary file for creating attachment (presumably) is
left around sometimes.
Bad...

Back to Bug 1595343 Comment 0