1993904 - Crash in [@ nsLocalFile::MoveOrCopyAsSingleFileOrDir]

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Gabriele Svelto [:gsvelto]]

Crash report: https://crash-stats.mozilla.org/report/index/b57aead6-569c-49e3-895e-19d5e0251012

MOZ_CRASH Reason:

MOZ_CRASH(NS_SUCCEEDED(IsDirectory(&isDir)))

Top 10 frames:

0  xul.dll  nsLocalFile::MoveOrCopyAsSingleFileOrDir(nsIFile*, nsTSubstring<char16_t> con...  xpcom/io/nsLocalFileWin.cpp:1872
1  xul.dll  nsLocalFile::RenameTo(nsIFile*, nsTSubstring<char16_t> const&)  xpcom/io/nsLocalFileWin.cpp:2317
2  xul.dll  nsLocalFile::RenameToNative(nsIFile*, nsTSubstring<char> const&)  xpcom/io/nsLocalFileWin.cpp:2326
3  xul.dll  mozilla::net::CacheFileIOManager::SyncRemoveAllCacheFiles::<lambda_8>::operat...  netwerk/cache2/CacheFileIOManager.cpp:4484
3  xul.dll  mozilla::net::CacheFileIOManager::SyncRemoveAllCacheFiles()  netwerk/cache2/CacheFileIOManager.cpp:4456
4  xul.dll  mozilla::net::CacheFileIOManager::Shutdown()  netwerk/cache2/CacheFileIOManager.cpp:1322
5  xul.dll  mozilla::net::CacheObserver::Observe(nsISupports*, char const*, char16_t const*)  netwerk/cache2/CacheObserver.cpp:225
6  xul.dll  nsObserverList::NotifyObservers(nsISupports*, char const*, char16_t const*)  xpcom/ds/nsObserverList.cpp:71
6  xul.dll  nsObserverService::NotifyObservers(nsISupports*, char const*, char16_t const*)  xpcom/ds/nsObserverService.cpp:288
7  xul.dll  mozilla::AppShutdown::AdvanceShutdownPhaseInternal(mozilla::ShutdownPhase, bo...  xpcom/base/AppShutdown.cpp:442

From what I can tell this appears to be regression introduced by the changes in bug 1978495. The crashes are only happening on Windows at the moment even though this does not appear to be a Windows-specific issue, at least on the surface.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1978495

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

This assertion is a windows-only assertion added by yannis recently, in bug 1980886, which landed the day before the crashes started.

Comment 3

[Kershaw Chang [:kershaw]]

(In reply to Randell Jesup [:jesup] (needinfo me) from comment #2)

This assertion is a windows-only assertion added by yannis recently, in bug 1980886, which landed the day before the crashes started.

I’ll change the component to match Bug 1980886.

Comment 4

[Yannis Juglaret [:yannis]]

Attachment: Bug 1993904 - Acknowledge a possible failure during file copy/move. r=#win-reviewers

We provide an alternate path if we can't determine whether the file we
are copying or moving is or isn't a directory. This information is only
required to potentially avoid a slow path, so in case we can't get it we
just continue with the copy/move operation and then unconditionally take
the slow path.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/22ca0db0b51b
https://hg.mozilla.org/integration/autoland/rev/621bb8c8b870
Acknowledge a possible failure during file copy/move. r=win-reviewers,gstoll

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/621bb8c8b870

Comment 7

[Yannis Juglaret [:yannis]]

Attachment: Bug 1993904 - Acknowledge a possible failure during file copy/move.

We provide an alternate path if we can't determine whether the file we
are copying or moving is or isn't a directory. This information is only
required to potentially avoid a slow path, so in case we can't get it we
just continue with the copy/move operation and then unconditionally take
the slow path.

Original Revision: https://phabricator.services.mozilla.com/D268845

Comment 8

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: Crashes in early beta builds, then potential incorrect behavior in late beta and release builds (in particular, downloaded files may get incorrect permissions like described in bug 1980886).
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: The patch only changes the bad code path, replacing a nigthly+early beta assertion by proper handling of the underlying failure. The normal code path is unchanged by this patch.
• String changes made/needed: No
• Is Android affected?: no

Comment 9

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/44b365fbed70
https://hg.mozilla.org/releases/mozilla-beta/rev/8758ab5debf3