Hi again,
I tried the solution that changes the if condition in
addNewNoDanglingRefs().
The code is now like this:
ObjectNameSet::iterator si;
ObjectMutationMap::iterator omuti;
ObjectMap::iterator omi;
CcbVector::iterator i = std::find_if(sCcbVector.begin(),
sCcbVector.end(), CcbIdIs(ccbId));
osafassert(i != sCcbVector.end());
CcbInfo* ccb = (*i);
for(si=dnSet.begin(); si!=dnSet.end(); ++si) {
omi = sObjectMap.find(*si);
// After all validation, object must exist
osafassert(omi != sObjectMap.end());
omuti = ccb->mMutations.find(*si);
*if (omuti != ccb->mMutations.end() && omuti->second->mOpType ==
IMM_CREATE) {*
sReverseRefsNoDanglingMMap.insert(std::pair<ObjectInfo *, ObjectInfo
*>(omi->second, obj));
}
}
But we still need 2 iterations because we can't free memory (delete
omut) in the loop.
addNewNoDanglingRefs() still needs the ObjectMutation to check the mOpType.
We have to free the memory in another loop.
Attached is the patch.
Best Regards,
Hung Nguyen
DEK Technologies
------------------------------------------------------------------------
*From:* Hung Nguyen
*Sent:* Tuesday, June 02, 2015 10:03AM
*To:* Zoran Milinkovic, Reddy Neelakanta Reddy Peddavandla, Anders
Bjornerstedt
*Cc:* opensaf-devel
*Subject:* Re: [devel] [PATCH 1 of 1] imm: Ensure IMM_MODIFY mutations
are always committed first [#1377]
Hi,
About this IF statement
if((omi->second->mObjFlags & IMM_CREATE_LOCK)||
omit!=ccb->mMutations.end())
If the OpType of the mutation is IMM_MODIFY, the statement will return
true and add duplicated NO_DANGLING reference to sReverseRefsNoDanglingMMap.
Here's the bug:
# immcfg
> immcfg -c TestClass testClass=1
> immcfg -c TestClass testClass=2
Add no-dangling ref
# immcfg
> immcfg -a dep=testClass=1 testClass=2
> immcfg -a dep=testClass=2 testClass=1
Remove no-dangling ref
# immcfg
> immcfg -a dep= testClass=1
> immcfg -a dep= testClass=2
The duplicated references are not removed so the object can't be deleted.
# immcfg -d testClass=1
error - saImmOmCcbApply FAILED: SA_AIS_ERR_FAILED_OPERATION (21)
# immcfg -d testClass=2
error - saImmOmCcbApply FAILED: SA_AIS_ERR_FAILED_OPERATION (21)
Best Regards,
Hung Nguyen
DEK Technologies
------------------------------------------------------------------------
*From:* Zoran Milinkovic
*Sent:* Monday, June 01, 2015 7:53PM
*To:* Reddy Neelakanta Reddy Peddavandla, Hung Nguyen, Anders
BjörnerstedtAnders Björnerstedt
*Cc:* opensaf-devel
*Subject:* RE: [devel] [PATCH 1 of 1] imm: Ensure IMM_MODIFY mutations
are always committed first [#1377]
Hi,
Anders and I have talked about the patch and had more-less same points as
Neelakanta mentioned in the review.
The patch has an impact on the performance, and it will affect every CCB (two
iterations), specially loading data.
The change could be done only for NO_DANGLING objects, changing
addNewNoDanglingRefs, as Neelakanta commented in his review.
Best regards,
Zoran
-----Original Message-----
From: Neelakanta Reddy [mailto:[email protected]]
Sent: Monday, June 01, 2015 2:33 PM
To: Hung Nguyen D; Anders Björnerstedt
Cc:[email protected]
Subject: Re: [devel] [PATCH 1 of 1] imm: Ensure IMM_MODIFY mutations are always
committed first [#1377]
Hi Hung,
Reviewed and tested the patch.
the patch is working good.
But the alternate solution may be passing ccbId, through commitModify
-->addNewNoDanglingRefs
In addNewNoDanglingRefs, modify the if condition (if CCB create is
committed and CREATE_LOCK is removed, then it will be present in
ccbvector of same CCB)
CcbInfo* ccb =(*(std::find_if(sCcbVector.begin(), sCcbVector.end(),
CcbIdIs(ccbId))));
ObjectMutationMap::iterator omit;
omit=omit.find(omi);
if((omi->second->mObjFlags & IMM_CREATE_LOCK)|| omit!=ccb->mMutations.end())
sReverseRefsNoDanglingMMap.insert(std::pair<ObjectInfo *, ObjectInfo
*>(omi->second, obj));
wait for AndersBj comments on alternate solution.
/Neel.
On Monday 01 June 2015 12:41 PM, Hung Nguyen wrote:
osaf/services/saf/immsv/immnd/ImmModel.cc | 65
+++++++++++++++++-------------
1 files changed, 36 insertions(+), 29 deletions(-)
When an IMM_CREATE mutation is committed, the IMM_CREATE_LOCK flag is also
cleared in commitCreate().
If the CCB has IMM_MODIFY mutations that add references to object of committed
IMM_CREATE mutations, it will fail to add NO_DANGLING references to
sReverseRefsNoDanglingMMap in addNewNoDanglingRefs() due to cleared
IMM_CREATE_LOCK flag.
mMutations map is sorted by object DN so the order of mutations to be committed
depends on the object DNs.
This patch ensures that IMM_MODIFY mutations of the CCB are always committed
first.
The mutation list will be looped through twice.
The first loop commits all IMM_MODIFY mutations, the second loop commits the
rest and free the allocated memory.
diff --git a/osaf/services/saf/immsv/immnd/ImmModel.cc
b/osaf/services/saf/immsv/immnd/ImmModel.cc
--- a/osaf/services/saf/immsv/immnd/ImmModel.cc
+++ b/osaf/services/saf/immsv/immnd/ImmModel.cc
@@ -5470,35 +5470,42 @@ ImmModel::ccbCommit(SaUint32T ccbId, Con
ccb->mWaitStartTime = 0;
//Do the actual commit!
- ObjectMutationMap::iterator omit;
- for(omit=ccb->mMutations.begin(); omit!=ccb->mMutations.end(); ++omit){
- ccbNotEmpty=true;
- ObjectMutation* omut = omit->second;
- osafassert(!omut->mWaitForImplAck);
- switch(omut->mOpType){
- case IMM_CREATE:
- if(ccbId != 1) {
- TRACE_5("COMMITING CREATE of %s", omit->first.c_str());
- }
- osafassert(omut->mAfterImage);
- commitCreate(omut->mAfterImage);
- omut->mAfterImage=NULL;
- break;
- case IMM_MODIFY:
- osafassert(omut->mAfterImage);
- pbeModeChange = commitModify(omit->first, omut->mAfterImage)
|| pbeModeChange;
- omut->mAfterImage=NULL;
- break;
- case IMM_DELETE:
- osafassert(omut->mAfterImage==NULL);
- commitDelete(omit->first);
- break;
- default:
- abort();
- }//switch
- delete omut;
- }//for
-
+ /* Ensure that modifications are committed first [#1377] */
+ for (int doIt = 0; doIt < 2; ++doIt) {
+ ObjectMutationMap::iterator omit;
+ for(omit=ccb->mMutations.begin(); omit!=ccb->mMutations.end(); ++omit){
+ ObjectMutation* omut = omit->second;
+ if ((!doIt && omut->mOpType != IMM_MODIFY) || (doIt &&
omut->mOpType == IMM_MODIFY)) {
+ if (doIt) delete omut; //only delete in the second loop
+ continue;
+ }
+ ccbNotEmpty=true;
+ osafassert(!omut->mWaitForImplAck);
+ switch(omut->mOpType){
+ case IMM_CREATE:
+ if(ccbId != 1) {
+ TRACE_5("COMMITING CREATE of %s", omit->first.c_str());
+ }
+ osafassert(omut->mAfterImage);
+ commitCreate(omut->mAfterImage);
+ omut->mAfterImage=NULL;
+ break;
+ case IMM_MODIFY:
+ osafassert(omut->mAfterImage);
+ pbeModeChange = commitModify(omit->first,
omut->mAfterImage) || pbeModeChange;
+ omut->mAfterImage=NULL;
+ break;
+ case IMM_DELETE:
+ osafassert(omut->mAfterImage==NULL);
+ commitDelete(omit->first);
+ break;
+ default:
+ abort();
+ }//switch
+ if (doIt) delete omut; //only delete in the second loop
+ }//for omit
+ }//for doIt
+
ccb->mMutations.clear();
if(ccbIdLongDnGuard == ccbId) {ccbIdLongDnGuard= 0;}
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
diff --git a/osaf/services/saf/immsv/immnd/ImmModel.cc
b/osaf/services/saf/immsv/immnd/ImmModel.cc
--- a/osaf/services/saf/immsv/immnd/ImmModel.cc
+++ b/osaf/services/saf/immsv/immnd/ImmModel.cc
@@ -5211,19 +5211,26 @@ ImmModel::removeNoDanglingRefs(ObjectInf
* "obj" will be inserted as a source object.
*/
void
-ImmModel::addNewNoDanglingRefs(ObjectInfo *obj, ObjectNameSet &dnSet)
+ImmModel::addNewNoDanglingRefs(ObjectInfo *obj, ObjectNameSet &dnSet,
SaUint32T ccbId)
{
ObjectNameSet::iterator si;
- ObjectMMap::iterator ommi;
+ ObjectMutationMap::iterator omuti;
ObjectMap::iterator omi;
-
- TRACE_ENTER();
+ CcbVector::iterator i;
+ CcbInfo* ccb = NULL;
+
+ TRACE_ENTER();
+
+ i = std::find_if(sCcbVector.begin(), sCcbVector.end(), CcbIdIs(ccbId));
+ osafassert(i != sCcbVector.end());
+ ccb = (*i);
for(si=dnSet.begin(); si!=dnSet.end(); ++si) {
omi = sObjectMap.find(*si);
// After all validation, object must exist
osafassert(omi != sObjectMap.end());
- if(omi->second->mObjFlags & IMM_CREATE_LOCK) {
+ omuti = ccb->mMutations.find(*si);
+ if (omuti != ccb->mMutations.end() && omuti->second->mOpType ==
IMM_CREATE) {
sReverseRefsNoDanglingMMap.insert(std::pair<ObjectInfo *,
ObjectInfo *>(omi->second, obj));
}
}
@@ -5329,7 +5336,7 @@ ImmModel::commitModify(const std::string
origNDRefs.begin(), origNDRefs.end(),
std::inserter(addedNDRefs, addedNDRefs.end()));
- addNewNoDanglingRefs(beforeImage, addedNDRefs);
+ addNewNoDanglingRefs(beforeImage, addedNDRefs, afterImage->mCcbId);
}
// sObjectMap.erase(oi);
@@ -5496,9 +5503,12 @@ ImmModel::ccbCommit(SaUint32T ccbId, Con
default:
abort();
}//switch
- delete omut;
}//for
+ for(omit=ccb->mMutations.begin(); omit!=ccb->mMutations.end(); ++omit){
+ delete omit->second;
+ }
+
ccb->mMutations.clear();
if(ccbIdLongDnGuard == ccbId) {ccbIdLongDnGuard= 0;}
diff --git a/osaf/services/saf/immsv/immnd/ImmModel.hh
b/osaf/services/saf/immsv/immnd/ImmModel.hh
--- a/osaf/services/saf/immsv/immnd/ImmModel.hh
+++ b/osaf/services/saf/immsv/immnd/ImmModel.hh
@@ -663,7 +663,8 @@ private:
bool removeRefsToObject = false);
void addNewNoDanglingRefs(
ObjectInfo *obj,
- ObjectNameSet &dnSet);
+ ObjectNameSet &dnSet,
+ SaUint32T ccbId);
void removeNoDanglingRefSet(
ObjectInfo *obj,
ObjectNameSet &dnSet);
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
