746731 - Uninitialised value use in nsHttpConnection::CanReuse

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Julian Seward [:jseward]]

/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)

Comment 1

[Patrick McManus [: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.

Comment 2

[Patrick McManus [:mcmanus]]

Attachment: patch 0

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.

Comment 3

[Patrick McManus [:mcmanus]]

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.

Comment 4

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 617502 [details] [diff] [review]
patch 0

[Triage Comment]
Merge of 14 to Aurora has completed.  Please renominate for mozilla-aurora.

Comment 5

[Patrick McManus [:mcmanus]]

This is FF15
https://hg.mozilla.org/integration/mozilla-inbound/rev/946995f07a3f

Comment 6

[Patrick McManus [:mcmanus]]

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.

Comment 7

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/946995f07a3f

Comment 8

[Alex Keybl [:akeybl]]

Comment on attachment 617502 [details] [diff] [review]
patch 0

[Triage Comment]
Regression in 14, approved for Aurora 14.

Comment 9

[Patrick McManus [:mcmanus]]

ff14
https://hg.mozilla.org/releases/mozilla-aurora/rev/00d9852452e5