Closed Bug 746731 Opened 8 years ago Closed 8 years ago

Uninitialised value use in nsHttpConnection::CanReuse

Categories

(Core :: Networking: HTTP, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla15
Tracking Status
firefox13 --- unaffected
firefox14 --- fixed
firefox15 --- fixed

People

(Reporter: jseward, Assigned: mcmanus)

Details

Attachments

(1 file)

/tests/toolkit/components/passwordmgr/test/test_bug_627616.html


(DISPLAY=:3.0 make -C ff-opt mochitest-plain TEST_PATH=toolkit/components/passwordmgr/test/test_bug_627616.html EXTRA_TEST_ARGS="--close-when-done --debugger=/home/sewardj/VgTRUNK/tchain/Inst/bin/valgrind --debugger-args='--smc-check=all-non-file --suppressions=/home/sewardj/MOZ/SUPPS/mochitest-mc.supp --error-limit=no --stats=no --trace-children=yes --child-silent-after-fork=yes "--trace-children-skip=/usr/bin/hg,/bin/rm,*/bin/certutil,*/bin/pk12util,*/bin/ssltunnel,*/bin/uname,*/bin/which,*/bin/ps,*/bin/grep" --tool=memcheck --track-origins=yes'") 2>&1 | tee spew-mc-627616-1


Conditional jump or move depends on uninitialised value(s)
   at 0x6315097: nsHttpConnection::CanReuse() (nsHttpConnection.cpp:571)
   by 0x631B839: nsHttpConnectionMgr::OnMsgReclaimConnection(int, void*) (nsHttpConnectionMgr.cpp:1895)
   by 0x6315DB7: nsHttpConnectionMgr::nsConnEvent::Run() (nsHttpConnectionMgr.h:518)
   by 0x6FF0E39: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:656)
   by 0x6FB95A9: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:245)
   by 0x62C303B: nsSocketTransportService::Run() (nsSocketTransportService2.cpp:652)
   by 0x6FF0E39: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:656)
   by 0x6FB95A9: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:245)
   by 0x6FF0B64: nsThread::ThreadFunc(void*) (nsThread.cpp:289)
   by 0x41A3EB2: _pt_root (ptthread.c:187)
   by 0x4A289C9: start_thread (pthread_create.c:300)
   by 0x53B470C: clone (clone.S:112)
 Uninitialised value was created by a heap allocation
   at 0x4029B9A: malloc (vg_replace_malloc.c:263)
   by 0x403CF6B: moz_xmalloc (mozalloc.cpp:87)
   by 0x631BB14: nsHttpConnectionMgr::nsHalfOpenSocket::OnOutputStreamReady(nsIAsyncOutputStream*) (mozalloc.h:229)
   by 0x62BE644: nsSocketOutputStream::OnSocketReady(unsigned int) (nsSocketTransport2.cpp:525)
   by 0x62C062C: nsSocketTransport::OnSocketReady(PRFileDesc*, short) (nsSocketTransport2.cpp:1568)
   by 0x62C2DB2: nsSocketTransportService::DoPollIteration(bool) (nsSocketTransportService2.cpp:762)
   by 0x62C3027: nsSocketTransportService::Run() (nsSocketTransportService2.cpp:645)
   by 0x6FF0E39: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:656)
   by 0x6FB95A9: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:245)
   by 0x6FF0B64: nsThread::ThreadFunc(void*) (nsThread.cpp:289)
   by 0x41A3EB2: _pt_root (ptthread.c:187)
   by 0x4A289C9: start_thread (pthread_create.c:300)
Assignee: nobody → mcmanus
Thanks Julian, somehow we lost the initializer when making that PRInterval based.. it matters in this case where you have a transaction that did not finish parsing the headers.

I also noted a problem that the default value that is used when parsing the headers is always the spdy one instead of the http/1 version in this case.

my bug.
Attached patch patch 0Splinter Review
Initialize mIdleTimeout to 0. This is the way it was before the printerval change - which means that any connection that does not make it to onheadersvailable will not be reused unless the timeout is set explicitly such as by connection manager when it has leftover connections from syn-retry. I considered making it the default http KA value instead of 0, but that should be a separate patch if we do that.

Also fix up one case where the default http/1 KA value should have been used but the spdy value was used instead.
Attachment #617502 - Flags: review?(honzab.moz)
Attachment #617502 - Flags: review?(honzab.moz) → review+
Comment on attachment 617502 [details] [diff] [review]
patch 0

[Approval Request Comment]
Please see https://wiki.mozilla.org/Tree_Rules for information on mozilla-central landings that do not require approval.

Possible risk to Fennec Native 14 (if any):

This fixes a regression in HTTP timeout handling that was introduced on FF14.
Attachment #617502 - Flags: approval-mozilla-central?
Comment on attachment 617502 [details] [diff] [review]
patch 0

[Triage Comment]
Merge of 14 to Aurora has completed.  Please renominate for mozilla-aurora.
Attachment #617502 - Flags: approval-mozilla-central?
Comment on attachment 617502 [details] [diff] [review]
patch 0

[Approval Request Comment]


This fixes a regression in HTTP timeout handling that was introduced on FF14.
Attachment #617502 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/946995f07a3f
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
Comment on attachment 617502 [details] [diff] [review]
patch 0

[Triage Comment]
Regression in 14, approved for Aurora 14.
Attachment #617502 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.