marionettejs http.Server should be close()-ed, not kill()-ed.

RESOLVED FIXED in 2.6 S6 - 1/29

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: sfoster, Assigned: sfoster)

Tracking

unspecified
2.6 S6 - 1/29
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [systemsfe])

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
There's a couple instances where we call .kill() on a http.Server instance. There is no kill method; we should call .close()
Created attachment 8709636 [details] [review]
[gaia] sfoster:close-server-bug-1240905 > mozilla-b2g:master
(Assignee)

Comment 2

3 years ago
Comment on attachment 8709636 [details] [review]
[gaia] sfoster:close-server-bug-1240905 > mozilla-b2g:master

I just found these 2 instances of this problem. This is code in /shared/ but I'm not sure its the correct component? Either way you seemed like an appropriate reviewer, but please forward it along if not.
Attachment #8709636 - Flags: review?(aus)
(Assignee)

Updated

3 years ago
Assignee: nobody → sfoster
Whiteboard: [systemsfe]
Target Milestone: --- → 2.6 S6 - 1/29

Comment 3

3 years ago
Comment on attachment 8709636 [details] [review]
[gaia] sfoster:close-server-bug-1240905 > mozilla-b2g:master

Looks good! I also checked and even in node 0.10 and 0.12, kill was removed. We must've been using a wrapper for a while and then replaced it with the straight up HTTP Server class from nodejs but failed to remove the use of 'kill' and instead use 'close'. I'm really surprised it didn't break things sooner! Sorry you had to step on that landmine but SUPER happy it's fixed!
Attachment #8709636 - Flags: review?(aus) → review+
(Assignee)

Comment 4

3 years ago
There's a lot of blue in the test run here. I'm not clear if this is normal - is this good to land? 

https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=7a25267bc249c852cfc7d3552f28327d5c41b2e7
Flags: needinfo?(mhenretty)
Yup, those blues are pretty normal these days:
https://treeherder.mozilla.org/#/jobs?repo=gaia-master
Flags: needinfo?(mhenretty)
(Assignee)

Comment 6

3 years ago
Merged to master: https://github.com/mozilla-b2g/gaia/commit/b7d18e723aba7c60fd15c9d0828f3e190853b752
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.