Bugfix: insidious dangling pointer caused by move after construction
Damn sideeffect of the suppport for move-only types: since we're moving our binding now into place /after/ construction, in some cases the end() iterator (embedded in RangeIter) becomes invalid. Indeed this was always broken, but didn't hurt, as long as we only used vectors. Solution: use a dedicated init() hook, which needs to be invoked *after* the TreeMutator has been constructed and moved into the final location in the stack buffer.
This commit is contained in:
parent
bada8ecffd
commit
1725a31df1
6 changed files with 42 additions and 5 deletions
|
|
@ -125,7 +125,6 @@ namespace ctrl{
|
|||
return false;
|
||||
else
|
||||
{
|
||||
UNIMPLEMENTED("actually apply a diff to the target Tangible");
|
||||
diff.applyTo (*entry->second);
|
||||
return true;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -369,12 +369,19 @@ namespace diff{
|
|||
TestWireTap(Target& dummy, PAR&& chain)
|
||||
: PAR{forward<PAR> (chain)}
|
||||
, target_(dummy)
|
||||
, pos_(target_.initMutation (identify(this)))
|
||||
, pos_()
|
||||
{ }
|
||||
|
||||
|
||||
|
||||
/* ==== re-Implementation of the operation API ==== */
|
||||
|
||||
virtual void
|
||||
init() override
|
||||
{
|
||||
pos_ = target_.initMutation (identify(this));
|
||||
PAR::init();
|
||||
}
|
||||
|
||||
/** record in the test target
|
||||
* that a new child element is
|
||||
|
|
|
|||
|
|
@ -379,7 +379,7 @@ namespace diff{
|
|||
using Target = typename TreeDiffTraits<TAR>::Ret;
|
||||
|
||||
Target target = mutatorBinding (subject_);
|
||||
buildMutator (target);
|
||||
buildMutator (target)->init();
|
||||
TreeDiffMutatorBinding::scopeManger_ = &scopes_;
|
||||
TreeDiffMutatorBinding::treeMutator_ = &scopes_.currentScope();
|
||||
REQUIRE (this->treeMutator_);
|
||||
|
|
|
|||
|
|
@ -268,7 +268,7 @@ namespace diff{
|
|||
ChildCollectionMutator(BIN&& wiringClosures, PAR&& chain)
|
||||
: PAR(std::forward<PAR>(chain))
|
||||
, binding_(forward<BIN>(wiringClosures))
|
||||
, pos_(binding_.initMutation())
|
||||
, pos_()
|
||||
{ }
|
||||
|
||||
|
||||
|
|
@ -276,6 +276,13 @@ namespace diff{
|
|||
|
||||
/* ==== Implementation of TreeNode operation API ==== */
|
||||
|
||||
virtual void
|
||||
init() override
|
||||
{
|
||||
pos_ = binding_.initMutation();
|
||||
PAR::init();
|
||||
}
|
||||
|
||||
/** fabricate a new element, based on
|
||||
* the given specification (GenNode),
|
||||
* and insert it at current position
|
||||
|
|
|
|||
|
|
@ -156,6 +156,15 @@ namespace diff{
|
|||
|
||||
/* ==== operation API ==== */
|
||||
|
||||
/** initialisation immediately before start of diff application
|
||||
* @remark allows for setup of state which is dependent on memory location,
|
||||
* like e.g. iterators. Due to the invokation via Builder DSL, the
|
||||
* implementation object may be moved after construction, but prior
|
||||
* to invoking this hook
|
||||
*/
|
||||
virtual void
|
||||
init() { }
|
||||
|
||||
virtual bool
|
||||
hasSrc () ////////////////////////////////////TODO questionable if we need it. Can not be sensibly implemented on multiple onion-layers!
|
||||
{
|
||||
|
|
|
|||
|
|
@ -169,6 +169,7 @@ namespace test{
|
|||
auto mutator =
|
||||
TreeMutator::build()
|
||||
.attachDummy (target);
|
||||
mutator.init();
|
||||
|
||||
CHECK (isnil (target));
|
||||
CHECK (not mutator.hasSrc());
|
||||
|
|
@ -203,6 +204,7 @@ namespace test{
|
|||
auto mutator2 =
|
||||
TreeMutator::build()
|
||||
.attachDummy (target);
|
||||
mutator2.init();
|
||||
|
||||
CHECK (target.verify("attachMutator")
|
||||
.beforeEvent("injectNew","78:56:34.012")
|
||||
|
|
@ -274,6 +276,7 @@ namespace test{
|
|||
auto mutator3 =
|
||||
TreeMutator::build()
|
||||
.attachDummy (target);
|
||||
mutator3.init();
|
||||
|
||||
// the first thing we try out is how to navigate through the sequence partially
|
||||
CHECK (isnil (target));
|
||||
|
|
@ -394,6 +397,8 @@ namespace test{
|
|||
|
||||
// --- first round: populate the collection ---
|
||||
|
||||
mutator1.init();
|
||||
|
||||
CHECK (isnil (target));
|
||||
CHECK (not mutator1.hasSrc());
|
||||
|
||||
|
|
@ -448,7 +453,7 @@ namespace test{
|
|||
// we have two lambdas now and thus can save on the size of one function pointer....
|
||||
CHECK (sizeof(mutator1) - sizeof(mutator2) == sizeof(void*));
|
||||
|
||||
|
||||
mutator2.init();
|
||||
CHECK (isnil (target)); // the "visible" new content is still void
|
||||
|
||||
CHECK (mutator2.matchSrc (ATTRIB1)); // current head element of src "matches" the given spec
|
||||
|
|
@ -558,6 +563,8 @@ namespace test{
|
|||
return true;
|
||||
}));
|
||||
|
||||
mutator3.init();
|
||||
|
||||
CHECK (isnil (target));
|
||||
CHECK (mutator3.matchSrc (ATTRIB1)); // new mutator starts out anew at the beginning
|
||||
CHECK (mutator3.accept_until (CHILD_T)); // fast forward behind the second-last child (CHILD_T) // accept_until
|
||||
|
|
@ -696,6 +703,7 @@ namespace test{
|
|||
LOG_SETTER ("gamma")
|
||||
gamma = val;
|
||||
});
|
||||
mutator1.init();
|
||||
|
||||
CHECK (sizeof(mutator1) <= sizeof(void*) // the TreeMutator VTable
|
||||
+ 2 * sizeof(void*) // one closure reference for each lambda
|
||||
|
|
@ -768,6 +776,8 @@ namespace test{
|
|||
LOG_SETTER ("gamma")
|
||||
gamma = val;
|
||||
});
|
||||
mutator2.init();
|
||||
|
||||
|
||||
CHECK (sizeof(mutator2) <= sizeof(void*) // the TreeMutator VTable
|
||||
+ 3 * sizeof(void*) // one closure reference for each lambda
|
||||
|
|
@ -857,6 +867,7 @@ namespace test{
|
|||
cout << "openSub()...\n"
|
||||
<< join(delta.getLog(), "\n") <<endl;
|
||||
});
|
||||
mutator3.init();
|
||||
|
||||
CHECK (sizeof(mutator1) <= sizeof(void*) // the TreeMutator VTable
|
||||
+ 2 * sizeof(void*) // one closure reference for each lambda
|
||||
|
|
@ -950,6 +961,7 @@ namespace test{
|
|||
auto mutator1 =
|
||||
TreeMutator::build()
|
||||
.attach (target);
|
||||
mutator1.init();
|
||||
|
||||
|
||||
#if false /////////////////////////////////////////////////////////////////////////////////////////////////////////////TICKET #1007 : strange behaviour, getting additional storage
|
||||
|
|
@ -1005,6 +1017,7 @@ namespace test{
|
|||
TreeMutator::build()
|
||||
.attach (target);
|
||||
|
||||
mutator2.init();
|
||||
|
||||
CHECK (isnil (target)); // old content moved aside, visible new content still void
|
||||
|
||||
|
|
@ -1075,6 +1088,8 @@ namespace test{
|
|||
TreeMutator::build()
|
||||
.attach (target);
|
||||
|
||||
mutator3.init();
|
||||
|
||||
CHECK (isnil (target));
|
||||
CHECK (mutator3.matchSrc (ATTRIB1)); // new mutator starts out anew at the beginning
|
||||
CHECK (mutator3.accept_until (CHILD_T)); // fast forward behind the second-last child (CHILD_T) // accept_until
|
||||
|
|
|
|||
Loading…
Reference in a new issue