Project

General

Profile

Bug #3608

Pit::erase crash if Interest name contains implicit digest

Added by Michael Sweatt about 4 years ago. Updated about 4 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

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

Actions
Copied from nTorrent - Bug #3607: NFD with Full NamesNew04/25/2016

Actions
#1

Updated by Michael Sweatt about 4 years ago

  • Copied from Bug #3607: NFD with Full Names added
#2

Updated by Alex Afanasyev about 4 years ago

  • Description updated (diff)
#3

Updated by Michael Sweatt about 4 years ago

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

#4

Updated by Spyros Mastorakis about 4 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!

#5

Updated by Alex Afanasyev about 4 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
#6

Updated by Junxiao Shi about 4 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.

#7

Updated by Michael Sweatt about 4 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.

#8

Updated by Junxiao Shi about 4 years ago

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

Updated by Junxiao Shi about 4 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.

#10

Updated by Junxiao Shi about 4 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.

#11

Updated by Weiwei Liu about 4 years ago

  • Status changed from New to In Progress
#12

Updated by Weiwei Liu about 4 years ago

  • Assignee changed from Weiwei Liu to Junxiao Shi
#13

Updated by Junxiao Shi about 4 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
#14

Updated by Junxiao Shi about 4 years ago

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

Updated by Junxiao Shi about 4 years ago

  • Status changed from Code review to Closed

Also available in: Atom PDF