Closed Bug 1313451 Opened 3 years ago Closed 3 years ago

Intermittent testGeckoRequest | application crashed [@ Zip::GetStream]

Categories

(Firefox for Android :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 52
Tracking Status
firefox51 --- fixed
firefox52 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: esawin)

Details

(Keywords: crash, intermittent-failure)

Attachments

(1 file, 1 obsolete file)

Eugen, is this something you'd have cycles to look at?
Flags: needinfo?(esawin)
Keywords: crash
This is interesting and it seems to have a useful stack.
Will take a look.
Assignee: nobody → esawin
Flags: needinfo?(esawin)
This might be related to bug 849589, bug 881564 and others which all seem to hit the "impossible" case which produces the following log:

11-03 15:22:45.581 28221 28238 E GeckoLinker: Not a seekable zstream
11-03 15:22:45.581 28221 28238 E GeckoLinker: Couldn't initialize SeekableZStream for libxul.so
11-03 15:22:45.581 28221 28238 E GeckoLinker: Not a seekable zstream
11-03 15:22:45.581 28221 28238 E GeckoLibLoad: Couldn't get a handle to libxul!
11-03 15:22:45.581 28221 28238 E GeckoLibLoad: Throw
11-03 15:22:45.583 28221 28221 D GeckoNetworkManager: New network state: UP, CELLULAR, CELL_2G
11-03 15:22:45.583 28221 28236 E GeckoJarReader: !!! BUG 849589 !!! origUrl=jar:jar:file:/data/app/org.mozilla.fennec-1/base.apk!/assets/omni.ja!/chrome/de-DE/locale/de-DE/browser/searchplugins/list.txt
11-03 15:22:45.583 28221 28236 E GeckoJarReader: java.lang.IllegalArgumentException: Got class java.util.zip.InflaterInputStream, but expected ByteBufferInputStream!
Since we can rule out a build issue (we would see this error more consistently in this case) something about the way we iterate through a zip directory must be broken.

For one, Zip::GetStream isn't thread safe although zips are "cached" and shared across threads now.

With this patch, we make Zip thread-safe by locking all access to nextFile, nextDir and entries members.
Attachment #8809486 - Flags: review?(mh+mozilla)
Comment on attachment 8809486 [details] [diff] [review]
0001-Bug-1313451-1.0-Make-Zip-thread-safe.-r-glandium.patch

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

::: mozglue/linker/Zip.cpp
@@ +65,4 @@
>  , nextDir(nullptr)
>  , entries(nullptr)
>  {
> +  pthread_mutex_init(&mutex, 0);

Please use nullptr.
Attachment #8809486 - Flags: review?(mh+mozilla) → review+
Addressed comment.
Attachment #8809486 - Attachment is obsolete: true
Attachment #8809781 - Flags: review+
Pushed by esawin@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bfd5335fa635
[1.1] Make Zip thread-safe. r=glandium
https://hg.mozilla.org/mozilla-central/rev/bfd5335fa635
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Please request Beta approval on this when you get a chance.
Flags: needinfo?(esawin)
Comment on attachment 8809781 [details] [diff] [review]
0001-Bug-1313451-1.1-Make-Zip-thread-safe.-r-glandium.patch

Approval Request Comment
[Feature/regressing bug #]: Native linker/zip reader.
[User impact if declined]: Crash on startup.
[Describe test coverage new/current, TreeHerder]: Nightly.
[Risks and why]: Minimal, it just adds mutex guards around some variable access.
[String/UUID change made/needed]: None.
Flags: needinfo?(esawin)
Attachment #8809781 - Flags: approval-mozilla-beta?
Attachment #8809781 - Flags: approval-mozilla-aurora?
Comment on attachment 8809781 [details] [diff] [review]
0001-Bug-1313451-1.1-Make-Zip-thread-safe.-r-glandium.patch

Fix an intermittent-failure. Beta51+ and Aurora52+. Should be in 51 beta 2.
Attachment #8809781 - Flags: approval-mozilla-beta?
Attachment #8809781 - Flags: approval-mozilla-beta+
Attachment #8809781 - Flags: approval-mozilla-aurora?
Attachment #8809781 - Flags: approval-mozilla-aurora+
Comment on attachment 8809781 [details] [diff] [review]
0001-Bug-1313451-1.1-Make-Zip-thread-safe.-r-glandium.patch

It's already fixed in 52. No need to uplift to Aurora52.
Attachment #8809781 - Flags: approval-mozilla-aurora+ → approval-mozilla-aurora-
You need to log in before you can comment on or make changes to this bug.