Bug #4168
closed
Interest::setNonce affects other instances sharing buffer
Added by Junxiao Shi over 7 years ago.
Updated over 7 years ago.
Description
Snippet to reproduce:
// g++ -std=c++11 -o x x.cpp $(pkg-config --cflags --libs libndn-cxx)
#include <ndn-cxx/interest.hpp>
#include <iostream>
int
main()
{
ndn::Interest i1("/A");
i1.setNonce(1);
i1.wireEncode();
std::cout << i1.getNonce() << ' ';
ndn::Interest i2(i1);
std::cout << i2.getNonce() << ' ';
i2.setNonce(2);
std::cout << i1.getNonce() << ' ';
std::cout << i2.getNonce() << '\n';
}
Expected: Output is 1 1 2 1.
Actual: Output is 1 1 2 2.
Interest::m_nonce is introduced in order to "make changing Nonce fast", however it causes this nasty problem.
This is not only premature optimization, but also can cause AsfStrategy, the only usage of setNonce within NFD, to react incorrectly to Nacks.
- When probing is due, AsfStrategy sends not only a normal Interest to the prefered nexthop from its measurements, but also a probing Interest to another interface.
The probing Interest is created by copying the normal Interest and calling
refreshNonce on the copy, and thus affected by this bug.
- Since all
Interest instances share the same wire encoding, the Interest stored on PIT in-record gains the random new Nonce.
- In case the node must respond a Nack to the downstream, the Nack would be created with the wrong Nonce.
Note that AsfStrategy does not currently send Nacks to downstream when Nacks are received from upstream, so this situation can only happen if strategy is changed to best-route at runtime.
- The downstream drops the Nack due to Nonce mismatch.
I think the solution is removing the premature optimization, and just resetting m_wire whenever setNonce is invoked.
Then, m_nonce can be a simple uint32_t quantity, so that getNonce() could be faster.
Also, it is no longer necessary to call wireDecode within wireEncode.
You can call everything "premature optimization". Is it known to cause a problem or just a conjecture? I'm not really in favor of resetting wire, but would not object the change.
- Assignee set to Junxiao Shi
Is it known to cause a problem or just a conjecture?
The problem can be triggered by following the steps in note-1. It's unlikely to happen in practice, but it would become a hard-to-debug problem if it happens when AsfStrategy is updated to return Nacks.
- Status changed from New to In Progress
- Status changed from In Progress to Code review
- % Done changed from 0 to 100
- Status changed from Code review to Closed
- Start date deleted (
07/06/2017)
Also available in: Atom
PDF
Updated by 
Updated by