Project

General

Profile

Actions

Bug #3608

closed

Pit::erase crash if Interest name contains implicit digest

Added by Michael Sweatt almost 8 years ago. Updated almost 8 years ago.

Status:
Closed
Priority:
High
Assignee:
Category:
Tables
Target version:
Start date:
04/25/2016
Due date:
% Done:

100%

Estimated time:
3.00 h

Description

NFD segfaults when receiving interests with full names.

(gdb) bt
#0  nfd::name_tree::Entry::erasePitEntry (this=0xa142a0, pitEntry=std::shared_ptr (count 6, weak 0) 0xac63c0)
    at ../daemon/table/name-tree-entry.cpp:103
#1  0x0000000000534312 in nfd::Pit::erase (this=this@entry=0x891ad8, 
    pitEntry=std::shared_ptr (count 6, weak 0) 0xac63c0) at ../daemon/table/pit.cpp:119
#2  0x00000000004ba91c in nfd::Forwarder::onInterestFinalize (this=0x891950, 
    pitEntry=std::shared_ptr (count 6, weak 0) 0xac63c0, isSatisfied=<optimized out>, dataFreshnessPeriod=...)
    at ../daemon/fw/forwarder.cpp:371
#3  0x00000000004c1d8f in boost::_mfi::mf3<void, nfd::Forwarder, std::shared_ptr<nfd::pit::Entry>, bool, boost::chrono::duration<long, boost::ratio<1l, 1000l> > const&>::operator() (a3=..., a2=<optimized out>, a1=..., 
    p=<optimized out>, this=<optimized out>) at /usr/include/boost/bind/mem_fn_template.hpp:393
#4  boost::_bi::list4<boost::_bi::value<nfd::Forwarder*>, boost::_bi::value<std::shared_ptr<nfd::pit::Entry> >, boost::_bi::value<bool>, boost::_bi::value<boost::chrono::duration<long, boost::ratio<1l, 1000l> > > >::operator()<boost::_mfi::mf3<void, nfd::Forwarder, std::shared_ptr<nfd::pit::Entry>, bool, boost::chrono::duration<long, boost::ratio<1l, 1000l> > const&>, boost::_bi::list0> (a=<synthetic pointer>, f=..., this=<optimized out>)
    at /usr/include/boost/bind/bind.hpp:457
#5  boost::_bi::bind_t<void, boost::_mfi::mf3<void, nfd::Forwarder, std::shared_ptr<nfd::pit::Entry>, bool, boost::chrono::duration<long, boost::ratio<1l, 1000l> > const&>, boost::_bi::list4<boost::_bi::value<nfd::Forwarder*>, boost::_bi::value<std::shared_ptr<nfd::pit::Entry> >, boost::_bi::value<bool>, boost::_bi::value<boost::chrono::duration<long, boost::ratio<1l, 1000l> > > > >::operator() (this=<optimized out>) at /usr/include/boost/bind/bind.hpp:893
#6  std::_Function_handler<void (), boost::_bi::bind_t<void, boost::_mfi::mf3<void, nfd::Forwarder, std::shared_ptr<nfd::pit::Entry>, bool, boost::chrono::duration<long, boost::ratio<1l, 1000l> > const&>, boost::_bi::list4<boost::_bi::value<nfd::Forwarder*>, boost::_bi::value<std::shared_ptr<nfd::pit::Entry> >, boost::_bi::value<bool>, boost::_bi::value<boost::chrono::duration<long, boost::ratio<1l, 1000l> > > > > >::_M_invoke(std::_Any_data const&) (
    __functor=...) at /usr/include/c++/5/functional:1871
#7  0x00007ffff7467722 in std::function<void ()>::operator()() const (this=0x7fffffffd710)
    at /usr/include/c++/5/functional:2271
#8  ndn::util::scheduler::Scheduler::onEvent (this=0x86f910, error=...) at ../src/util/scheduler.cpp:182
#9  0x00007ffff7467e10 in std::_Mem_fn_base<void (ndn::util::scheduler::Scheduler::*)(boost::system::error_code const&), true>::operator()<boost::system::error_code const&, void> (__object=<optimized out>, this=0x7fffffffd7a0)
    at /usr/include/c++/5/functional:600
#10 std::_Bind<std::_Mem_fn<void (ndn::util::scheduler::Scheduler::*)(boost::system::error_code const&)> (ndn::util::scheduler::Scheduler*, std::_Placeholder<1>)>::__call<void, boost::system::error_code const&, 0ul, 1ul>(std::tuple<boost::system::error_code const&>&&, std::_Index_tuple<0ul, 1ul>) (__args=<optimized out>, this=0x7fffffffd7a0)
    at /usr/include/c++/5/functional:1074
#11 std::_Bind<std::_Mem_fn<void (ndn::util::scheduler::Scheduler::*)(boost::system::error_code const&)> (ndn::util::scheduler::Scheduler*, std::_Placeholder<1>)>::operator()<boost::system::error_code const&, void>(boost::system::error_code const&) (this=0x7fffffffd7a0) at /usr/include/c++/5/functional:1133
#12 boost::asio::detail::binder1<std::_Bind<std::_Mem_fn<void (ndn::util::scheduler::Scheduler::*)(boost::system::error_code const&)> (ndn::util::scheduler::Scheduler*, std::_Placeholder<1>)>, boost::system::error_code>::operator()()
    (this=0x7fffffffd7a0) at /usr/include/boost/asio/detail/bind_handler.hpp:47
#13 boost::asio::asio_handler_invoke<boost::asio::detail::binder1<std::_Bind<std::_Mem_fn<void (ndn::util::scheduler::Scheduler::*)(boost::system::error_code const&)> (ndn::util::scheduler::Scheduler*, std::_Placeholder<1>)>, boost::system::error_code> >(boost::asio::detail::binder1<std::_Bind<std::_Mem_fn<void (ndn::util::scheduler::Scheduler::*)(boost::system::error_code const&)> (ndn::util::scheduler::Scheduler*, std::_Placeholder<1>)>, boost::system::error_code>&, ...) (function=...) at /usr/include/boost/asio/handler_invoke_hook.hpp:69
#14 boost_asio_handler_invoke_helpers::invoke<boost::asio::detail::binder1<std::_Bind<std::_Mem_fn<void (ndn::util::scheduler::Scheduler::*)(boost::system::error_code const&)> (ndn::util::scheduler::Scheduler*, std::_Placeholder<1>)>, boost::system::error_code>, std::_Bind<std::_Mem_fn<void (ndn::util::scheduler::Scheduler::*)(boost::system::error_code const&)> (ndn::util::scheduler::Scheduler*, std::_Placeholder<1>)> >(boost::asio::detail::binder1<std::_Bind<std::_Mem_fn<void (ndn::util::scheduler::Scheduler::*)(boost::system::error_code const&)> (ndn::util::scheduler::Schedule---Type <return> to continue, or q <return> to quit---

Files

consumer.cpp (1.17 KB) consumer.cpp The consumer Michael Sweatt, 04/25/2016 02:59 PM
producer.cpp (1.3 KB) producer.cpp The producer Michael Sweatt, 04/25/2016 02:59 PM
data.txt (126 Bytes) data.txt A data packet that was failing Michael Sweatt, 04/25/2016 02:59 PM

Related issues 1 (0 open1 closed)

Related to NFD - Bug #3363: Interest with full name is not satisfied with Data packetClosedJunxiao Shi

Actions
Actions #2

Updated by Alex Afanasyev almost 8 years ago

  • Description updated (diff)
Actions #3

Updated by Michael Sweatt almost 8 years ago

OS: Ubuntu 15.10
NFD and ndn-cxx direct from apt-get (dbg versions of each)

Actions #4

Updated by Spyros Mastorakis almost 8 years ago

@Junxiao: It would be extremely helpful if you could take personal care of this one, since it is blocking the development of the entire nTorrent application.

Thanks!

Actions #5

Updated by Alex Afanasyev almost 8 years ago

  • Category set to Forwarding
  • Assignee set to Junxiao Shi
  • Target version set to v0.5

Confirmed on OS X 10.11, master branch of NFD and ndn-cxx.

    frame #4: 0x000000010022cca9 nfd`nfd::name_tree::Entry::erasePitEntry(this=0x000000010391cca8, pitEntry=std::__1::shared_ptr<nfd::pit::Entry>::element_type @ 0x0000000101d20628 strong=6 weak=1) + 233 at name-tree-entry.cpp:97
    frame #5: 0x0000000100244032 nfd`nfd::Pit::erase(this=0x0000000103c03aa8, pitEntry=std::__1::shared_ptr<nfd::pit::Entry>::element_type @ 0x0000000101d20628 strong=6 weak=1) + 306 at pit.cpp:119
    frame #6: 0x00000001000f803c nfd`nfd::Forwarder::onInterestFinalize(this=0x0000000103c03930, pitEntry=std::__1::shared_ptr<nfd::pit::Entry>::element_type @ 0x0000000101d20628 strong=6 weak=1, isSatisfied=true, dataFreshnessPeriod=0x0000000101d22888) + 972 at forwarder.cpp:366
Actions #6

Updated by Junxiao Shi almost 8 years ago

Steps to reproduce is missing.
In particular: In what order are the consumer and producer programs executed?

Also, notice that Ubuntu 15.10 is not a supported platform, but OSX 10.11 is supported.

Actions #7

Updated by Michael Sweatt almost 8 years ago

1) Run producer

2) Run consumer

Fix it on supported platforms if it does not work for 15.10 I will follow up.

Actions #8

Updated by Junxiao Shi almost 8 years ago

  • Related to Bug #3363: Interest with full name is not satisfied with Data packet added
Actions #9

Updated by Junxiao Shi almost 8 years ago

  • Subject changed from NFD with Full Names to Pit::erase crash if Interest name contains implicit digest
  • Category changed from Forwarding to Tables
  • Estimated time set to 3.00 h

I'm able to reproduce this issue on OSX 10.11. The error message I get is:

Assertion failed: (pitEntry->m_nameTreeEntry.get() == this), function erasePitEntry, file ../daemon/table/name-tree-entry.cpp, line 97.

This is caused by a mistake in Pit::erase:

shared_ptr<name_tree::Entry> nameTreeEntry = m_nameTree.get(*pitEntry);
(omit)
nameTreeEntry->erasePitEntry(pitEntry);

nameTree.get(pitEntry) is equivalent to nameTree.lookup(pitEntry.getName()), which creates a new NameTree entry whose name contains the implicit digest, instead of returning the existing NameTree entry that the PIT entry is attached onto.

Afterwards, when the code attempts to erase the PIT entry from the NameTree entry, the assertion "PIT entry is attached to this NameTree entry" fails.

The quick fix is to change the nameTree.get call into pitEntry->m_nameTreeEntry.
However, it's unacceptable for long term use because m_nameTreeEntry field should only be accessed from NameTree class, per NameTree design.

The proper fix is to introduce another NameTree API that returns the attach point of a PIT entry.


In 20160426 conference call, I promised to "have a look" at this issue this week, and the above note fulfills this promise.

Presently I have a backlog of tasks, so I probably won't have time to write the patch and test case until after May 15.

Actions #10

Updated by Junxiao Shi almost 8 years ago

  • Assignee changed from Junxiao Shi to Weiwei Liu
  • Priority changed from Normal to High

Since @Spyros wants this sooner than what I could promise, I have to reassign this issue to @Weiwei. Hopefully she can complete this earlier.

Actions #11

Updated by Weiwei Liu almost 8 years ago

  • Status changed from New to In Progress
Actions #12

Updated by Weiwei Liu almost 8 years ago

  • Assignee changed from Weiwei Liu to Junxiao Shi
Actions #13

Updated by Junxiao Shi almost 8 years ago

  • % Done changed from 0 to 40

http://gerrit.named-data.net/2867 is the quick fix NFD:commit:704430ca07bda92019538b92379c276529efa04a rebased onto master, with minimal changes.

The next commit will make the following changes:

  1. rename NameTree::get(const xx::Entry&) to NameTree::lookup(const xx::Entry&), to be consistent with NameTree::lookup(Name) and better reflect semantics
  2. introduce NameTree::getEntry(const xx::Entry&) that returns the NameTreeEntry on which a table entry is attached
  3. change xx::Entry::m_nameTreeEntry from shared_ptr to weak_ptr, which avoids circular reference and prevents potential memory leak
Actions #14

Updated by Junxiao Shi almost 8 years ago

  • Status changed from In Progress to Code review
  • % Done changed from 40 to 100
Actions #15

Updated by Junxiao Shi almost 8 years ago

  • Status changed from Code review to Closed
Actions

Also available in: Atom PDF