Closed Bug 1238010 Opened 4 years ago Closed 4 years ago

Turn off ClosingService

Categories

(Core :: Networking, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox44 --- fixed
firefox45 --- fixed
firefox46 --- fixed
b2g-v2.5 --- fixed

People

(Reporter: dragana, Assigned: dragana, NeedInfo)

References

Details

Attachments

(1 file, 1 obsolete file)

ClosingService is doing PR_Close on a separate thread, because of some shutdown crashes that we have.

But it seems to be incompatible with some dll-s so we are removing it - bug 1233237.
Turning off ClosingService fix the problem from bug 1233237: https://bugzilla.mozilla.org/show_bug.cgi?id=1233237#c55

This bug it is just going to turn it of so that it is not used and we want to uplift this to aurora an beta.
Assignee: nobody → dd.mozilla
Blocks: 1233237
Status: NEW → ASSIGNED
Attached patch bug_1238010.patch (obsolete) — Splinter Review
Attachment #8705748 - Flags: review?(mcmanus)
Comment on attachment 8705748 [details] [diff] [review]
bug_1238010.patch

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

::: netwerk/base/ClosingService.cpp
@@ +121,5 @@
>  // static
>  nsresult
>  ClosingService::AttachIOLayer(PRFileDesc *aFd)
>  {
> +  return NS_OK;

comment please :)
Attachment #8705748 - Flags: review?(mcmanus) → review+
Added a comment
Attachment #8705748 - Attachment is obsolete: true
Attachment #8705858 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1b3dec8ee549
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Comment on attachment 8705858 [details] [diff] [review]
bug_1238010.patch

Approval Request Comment
[Feature/regressing bug #]: Bug 1152046
[User impact if declined]: Crash with certain LSP - bug 1233237, https://bugzilla.mozilla.org/show_bug.cgi?id=1233237#c48 . We are not sure if LSP are thread safe so we want to remove ClosingService completely(bug 1238017). This bug is just going to turn ClosingService off.
[Describe test coverage new/current, TreeHerder]: One reporter could reproduce it reliably and he tested a version with ClosingService removed: https://bugzilla.mozilla.org/show_bug.cgi?id=1233237#c55
[Risks and why]: Low - we just early return from a function (we just do not attache an additional layer to sockets so ClosingService will never be called).
[String/UUID change made/needed]: none
Attachment #8705858 - Flags: approval-mozilla-beta?
Attachment #8705858 - Flags: approval-mozilla-aurora?
DavidFR, HCT: You were able to repro the crash in bug 1233237 (comment 44 and 39). Could you please verify the crash is gone with the latest Nightly build (it has a fix)? 

Your verification will be very valuable in helping us decide whether the fix works and to include it in our upcoming Fx44 release. Thanks!
Flags: needinfo?(hctamtb)
Flags: needinfo?(davidfr)
Comment on attachment 8705858 [details] [diff] [review]
bug_1238010.patch

This will help fix shutdown hangs. As stability fixes do meet the Beta44 uplift criteria and the fix is a one-liner, I am taking it in Beta44, Aurora45.
Attachment #8705858 - Flags: approval-mozilla-beta?
Attachment #8705858 - Flags: approval-mozilla-beta+
Attachment #8705858 - Flags: approval-mozilla-aurora?
Attachment #8705858 - Flags: approval-mozilla-aurora+
Hello,
Thanks for your work !
The last Nightly build is fixed :-) Internet work and firefox don't crash
Flags: needinfo?(davidfr)
You need to log in before you can comment on or make changes to this bug.