Closed Bug 1269185 Opened 4 years ago Closed 4 years ago
startup crash in ns
Zip Handle::find Data Start
[Tracking Requested - why for this release]: nominating for tracking since it's a recent regression This bug was filed from the Socorro interface and is report bp-43fd92e8-1556-4540-ac47-891212160428. ============================================================= Crashing Thread (0) Frame Module Signature Source 0 xul.dll nsZipHandle::findDataStart() modules/libjar/nsZipArchive.cpp 1 xul.dll nsZipHandle::Init(nsIFile*, nsZipHandle**, PRFileDesc**) modules/libjar/nsZipArchive.cpp 2 xul.dll nsZipArchive::OpenArchive(nsIFile*) modules/libjar/nsZipArchive.cpp 3 xul.dll mozilla::scache::StartupCache::LoadArchive(mozilla::scache::StartupCache::TelemetrifyAge) startupcache/StartupCache.cpp 4 xul.dll mozilla::scache::StartupCache::Init() startupcache/StartupCache.cpp 5 xul.dll mozilla::scache::StartupCache::InitSingleton() startupcache/StartupCache.cpp 6 xul.dll mozilla::scache::StartupCache::GetSingleton() startupcache/StartupCache.cpp 7 xul.dll NS_InitXPCOM2 xpcom/build/XPCOMInit.cpp 8 xul.dll ScopedXPCOMStartup::Initialize() toolkit/xre/nsAppRunner.cpp 9 xul.dll XREMain::XRE_main(int, char** const, nsXREAppData const*) toolkit/xre/nsAppRunner.cpp 10 xul.dll XRE_main toolkit/xre/nsAppRunner.cpp 11 xul.dll nsAString_internal::ReplacePrep(unsigned int, unsigned int, unsigned int) xpcom/string/nsTSubstring.cpp 12 firefox.exe firefox.exe@0x151d3 this new crash signature starts to show up in 47.0b1 and it's occurring on startup. it looks related to the work done in bug 1260836.
In some parts of libjar, we use Windows SEH to trap reads from mapped memory and turn those into errors: see e.g. https://hg.mozilla.org/releases/mozilla-beta/annotate/5bbf2e7c2fc6/modules/libjar/nsZipArchive.cpp#l447 I don't know whether, during this refactoring, you just moved an existing crash to another function (which is probably not a big deal). But if you moved access outside of an SEH block, it may be simple to re-add that and propagate errors instead of crashing. Antivirus is a common cause for memory-mapped files to "disappear" causing these IN_PAGE_ERROR exceptions.
That explains a lot. I was just in the middle of writing a comment noting how these crashes have been seen before (in a different function), but mostly in OSX whereas now we are seeing them in Windows. I will write a patch to add the windows exception handling. Currently, all crashes with the signature nsZipHandle::findDataStart are in Windows, so this should fix that problem. However, I feel that it is worth pointing out that this bug still gets hit in other operating systems. Before I added nsZipHandle::findDataStart, the first place where the zip file is read is in nsZipArchive::BuildFileList . Looking for crashes with this signature yields a number of crashes in OSX . These have the same stack trace, just crashing in nsZipArchive::BuildFileList instead of nsZipHandle::findDataStart.  https://hg.mozilla.org/releases/mozilla-release/annotate/e35da3da61cb/modules/libjar/nsZipArchive.cpp#l625  https://crash-stats.mozilla.com/report/index/6ebcea83-ce6b-481f-b14b-181572160428
Review commit: https://reviewboard.mozilla.org/r/50551/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/50551/
Attachment #8748827 - Flags: review?(spohl.mozilla.bugs)
Comment on attachment 8748827 [details] MozReview Request: Bug 1269185 - Prevent crashes in Windows when zip files cannot be read https://reviewboard.mozilla.org/r/50551/#review47517
Attachment #8748827 - Flags: review?(spohl.mozilla.bugs) → review+
Any interest in uplifting this, given that Bug 1260836 (Add support for CRX) was uplifted?
(In reply to Kirk Steuber [:ksteuber] from comment #5) > Any interest in uplifting this, given that Bug 1260836 (Add support for CRX) > was uplifted? Yes! Thanks for the ni?
Comment on attachment 8748827 [details] MozReview Request: Bug 1269185 - Prevent crashes in Windows when zip files cannot be read Approval Request Comment [Feature/regressing bug #]: Widevine EME [User impact if declined]: Crashes when we fail to install the Widevine CDM [Describe test coverage new/current, TreeHerder]: We have tests covering the Widevine CDM installer [Risks and why]: Low; just adding error checking [String/UUID change made/needed]: None.
Comment on attachment 8748827 [details] MozReview Request: Bug 1269185 - Prevent crashes in Windows when zip files cannot be read Fix a crash during Widevine CDM installation, Aurora48+, Beta47+
You need to log in before you can comment on or make changes to this bug.