New issue
Advanced search Search tips

Issue 863471 link

Starred by 2 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug



Sign in to add a comment

Potential memory leak in HpackHeaderTable and HpackEntry

Project Member Reported by erikc...@chromium.org, Jul 13

Issue description

The "leaks" tool on macOS keeps track of every call to malloc, and then searches the entire heap/stack/registers for references to that pointer. If no reference is found, that call to malloc is determined to be a leak.

The "leaks" tool, when run on both debug & release builds of Chrome seems to *really* strongly think that there's a leak in HpackHeaderTable. ~46 leaks after a couple of navigations to gmail & youtube. On a long running instance of Chrome [no symbols], it's finding many more leaks whose contents strongly resembles headers of some type of network request.

On a debug build, the call to malloc has a similar stack trace:
"""
  Call stack: [thread 0x700003332000]: |                                        
  thread_start |                                                                
  _pthread_body |                                                               
  _pthread_body |                                                               
  base::(anonymous namespace)::ThreadFunc(void*) platform_thread_posix.cc:78 |  
  base::Thread::ThreadMain() thread.cc:340 |                                    
  content::BrowserProcessSubThread::Run(base::RunLoop*) browser_process_sub_thread.cc:129 |
  content::BrowserProcessSubThread::IOThreadRun(base::RunLoop*) browser_process_sub_thread.cc:179 |
  base::Thread::Run(base::RunLoop*) thread.cc:255 |                             
  base::RunLoop::Run() run_loop.cc:105 |                                        
  base::MessageLoop::Run(bool) message_loop.cc:0 |                              
  base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) message_pump_libevent.cc:210 |
  base::MessageLoop::DoWork() message_loop.cc:535 |                             
  base::MessageLoop::DeferOrRunPendingTask(base::PendingTask) message_loop.cc:468 |
  base::MessageLoop::RunTask(base::PendingTask*) message_loop.cc:456 |          
  base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) task_annotator.cc:103 |
  base::OnceCallback<void ()>::Run() && callback.h:100 |                        
  base::internal::Invoker<base::internal::BindState<void (net::SpdySession::*)(net::SpdySession::WriteState, int), base::WeakPtr<net::SpdySession>, net::SpdySession::WriteState, net::Error>, void ()>::Run(base::internal::BindStateBase*) bind_internal.h:662 |
  void base::internal::Invoker<base::internal::BindState<void (net::SpdySession::*)(net::SpdySession::WriteState, int), base::WeakPtr<net::SpdySession>, net::SpdySession::WriteState, net::Error>, void ()>::RunImpl<void (net::SpdySession::* const&)(net::SpdySession::WriteState, int), std::__1::tuple<base::WeakPtr<net::SpdySession>, net::SpdySession::WriteState, net::Error> const&, 0ul, 1ul, 2ul>(void (net::SpdySession::* const&&&)(net::SpdySession::WriteState, int), std::__1::tuple<base::WeakPtr<net::SpdySession>, net::SpdySession::WriteState, net::Error> const&&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul>) bind_internal.h:680 |
  void base::internal::InvokeHelper<true, void>::MakeItSo<void (net::SpdySession::* const&)(net::SpdySession::WriteState, int), base::WeakPtr<net::SpdySession> const&, net::SpdySession::WriteState const&, net::Error const&>(void (net::SpdySession::* const&&&)(net::SpdySession::WriteState, int), base::WeakPtr<net::SpdySession> const&&&, net::SpdySession::WriteState const&&&, net::Error const&&&) bind_internal.h:630 |
  void base::internal::FunctorTraits<void (net::SpdySession::*)(net::SpdySession::WriteState, int), void>::Invoke<void (net::SpdySession::*)(net::SpdySession::WriteState, int), base::WeakPtr<net::SpdySession> const&, net::SpdySession::WriteState const&, net::Error const&>(void (net::SpdySession::*)(net::SpdySession::WriteState, int), base::WeakPtr<net::SpdySession> const&&&, net::SpdySession::WriteState const&&&, net::Error const&&&) bind_internal.h:507 |
  net::SpdySession::PumpWriteLoop(net::SpdySession::WriteState, int) spdy_session.cc:2100 |
  net::SpdySession::DoWriteLoop(net::SpdySession::WriteState, int) spdy_session.cc:2130 |
  net::SpdySession::DoWrite() spdy_session.cc:2194 |                            
  net::SpdyStream::HeadersBufferProducer::ProduceBuffer() spdy_stream.cc:76 |   
  net::SpdyStream::ProduceHeadersFrame() spdy_stream.cc:196 |                   
  net::SpdySession::CreateHeaders(unsigned int, net::RequestPriority, spdy::SpdyControlFlags, spdy::SpdyHeaderBlock, net::NetLogSource) spdy_session.cc:1019 |
  net::BufferedSpdyFramer::SerializeFrame(spdy::SpdyFrameIR const&) buffered_spdy_framer.h:225 |
  spdy::SpdyFramer::SerializeFrame(spdy::SpdyFrameIR const&) spdy_framer.cc:917 |
  spdy::SpdyHeadersIR::Visit(spdy::SpdyFrameVisitor*) const spdy_protocol.cc:425 |
  spdy::(anonymous namespace)::FrameSerializationVisitor::VisitHeaders(spdy::SpdyHeadersIR const&) spdy_framer.cc:807 |
  spdy::SpdyFramer::SerializeHeaders(spdy::SpdyHeadersIR const&) spdy_framer.cc:620 |
  spdy::SpdyFramer::SerializeHeadersBuilderHelper(spdy::SpdyHeadersIR const&, unsigned char*, unsigned long*, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*, int*, unsigned long*) spdy_framer.cc:586 |
  spdy::HpackEncoder::EncodeHeaderSet(spdy::SpdyHeaderBlock const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*) hpack_encoder.cc:113 |
  spdy::HpackEncoder::EncodeRepresentations(spdy::HpackEncoder::RepresentationIterator*, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*) hpack_encoder.cc:146 |
  spdy::HpackEncoder::EmitIndexedLiteral(std::__1::pair<base::BasicStringPiece<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, base::BasicStringPiece<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&) hpack_encoder.cc:168 |
  spdy::HpackHeaderTable::TryAddEntry(base::BasicStringPiece<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, base::BasicStringPiece<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >) hpack_header_table.cc:197 |
  std::__1::deque<spdy::HpackEntry, std::__1::allocator<spdy::HpackEntry> >::push_front(spdy::HpackEntry&&) deque:1884 |
  spdy::HpackEntry::HpackEntry(spdy::HpackEntry const&) hpack_entry.cc:50 |     
  spdy::HpackEntry::HpackEntry(spdy::HpackEntry const&) hpack_entry.cc:46 |     
  std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::operator=(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) |
  std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::assign(char const*, unsigned long) |
  std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::__grow_by_and_replace(unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, char const*) |
  operator new(unsigned long) | malloc | malloc_zone_malloc      
"""

The leaks always seem to think that it's |value_| that's leaking, not |name_|. Nothing seems suspicious about HpackEntry itself. This makes me think something weird is happening in HpackHeaderTable, probably involving "dynamic_entries_".

To debug:
1) Build Chrome on macOS with symbols
2) set environment variable: "export MallocStackLogging=1"
3) Launch Chrome from the same terminal as (2)
4) Navigate to gmail.com, log in.
5) Run "leaks <pid>" where <pid> is for the browser process.
 
Cc: b...@chromium.org morlovich@chromium.org
Labels: OS-Mac
To clarify, the tool is saying the buffer allocated by |value_| is leaking. If the underlying string |value_| is still around, it should be pointing to its own internal buffer and not be classified as a leak, as long as the buffer wasn't swapped out. Since it doesn't look like HpackEntry does anything like that with its string, I'm not sure how this could be happening, but I could be missing something.

There is something strange going on with HpackEntry::operator= though. If the type if |this| is not LOOKUP but the type of |other| is, we don't clear storage for |value_| and |name_| when maybe we should. I don't think that could cause a true leak though.
Components: -Internals>Network Internals>Network>QUIC Internals>Network>HTTP2

Sign in to add a comment