Re: [Boost-users] [multi_index] Problem of scope with the replace in static data structure

Hi Joaquim,
It's hard to know whether you got copy semantics right without knowing the types of _rangeBegin, _rangeEnd and the rest of Graph data members, but this has prompted me to suspect that maybe there's something wrong with the handler type Graph_h (which I assume is some sort of ref-counted reference to Graph, right?). Can you please test the following? In your //Test of the commit section, rather than reuse graph_h please use a different variable to do the after-commitChanges check:
//Test of the commit if (graph_h.commitChanges()) cout << "Commit was successful!\n"; else cout << "Commitretr was unsuccessful!\n"; // Debug
//Graph_h graph_h; //NodeIterator nodeIt; //Node_h node_h;
graphIt it = DynamicGraphs::_graphRep.find("COI:0351-123-4555"); Graph_h graph_h2 = Graph_h(it); // WE DON'T REUSE graph_h //propertystream_h propS; propS = graph_h2->lookupLocalProp("name"); p_h = propS.next(); cout << "DEBUG@main:right after the commit:_git " << p_h.value() <<endl; nodeIt = graph_h2->nodeIterator("Node:0351-123-4569"); if (!nodeIt) cout << "Node version was not found!"<<endl; else //Works for both: one match and range lookup while (!nodeIt.end()){ node_h = nodeIt.next(); cout << "Found it: " << *node_h; } //Debug
Does this make any difference?
Yes, it worked, if I instantiate another object (like graph_h2). After that I have done some more Debug to try to find the reason, but I still don't understand,because inside the constructor it finds the Nodes, when I use the same object (graph_h)... Inside the overloaded operator-> does not work anymore, even I am using the _localGraph in both stretch of codes. So if it was the assignment of the _localGraph, it should not work with the constructor also, isn't true? And the commitChanges, that I told before, it is for replacing this _localGraph in the repository of graphs graphIndex _graphRep; For better explanation, here it is the code: Remark: the Graph_h it will be using referencing count, but since I don't know how yet, it is not implemented. So the Graph_h I use as way of the user handle the Graph, so it has as members: - the iterator of the lookup for the Graph - _localGraph: it is where I can change the Graph, because in the index it is not possible, since their elements are constant - and bool to indicate the creation success /// Graph Handler: Wrapper for manipulating the Graph objects class Graph_h : virtual Handler { private: graphIt _git; Graph _localGraph; bool _createSuccess; public: /// Default Constructor Graph_h() : _createSuccess(false){} /// Constructor with Parameters: for creation of Graphs Graph_h (const pair<graphIt,bool> &creation) : _git(creation.first), _createSuccess(creation.second){ _localGraph = _git->_graph; } ///////////////The below constructor it is where it works,//////////// ///////////////the retrieve of nodes with _localGraph///////////// /// Constructor with Parameters: for lookup of Graphs Graph_h (const graphIt &it) : _git(it), _createSuccess(false){ _localGraph = it->_graph; //#ifndef NDEBUG_DG //Graph_h graph_h; NodeIterator nodeIt; Node_h node_h; //graphIt it = DynamicGraphs::_graphRep.find("COI:0351-123-4555"); //Graph_h //graph_h = Graph_h(it); propertystream_h propS; property_h p_h; propS = _localGraph.lookupLocalProp("name"); p_h = propS.next(); cout << "DEBUG@Graph_hConstructor(it) " << p_h.value() <<endl; nodeIt = _localGraph.nodeIterator("Node:0351-123-4569"); if (!nodeIt) cout << "Node version was not found!"<<endl; else //Works for both: one match and range lookup while (!nodeIt.end()){ node_h = nodeIt.next(); cout << "Found it: " << *node_h; } //#endif //NDEBUG_DG } /// Copy Constructor Graph_h (const Graph_h &h) : _git(h._git), _createSuccess(h._createSuccess){ _localGraph = h._localGraph; } Graph_h& operator=(const Graph_h &h) { _git = h._git; _createSuccess = h._createSuccess; _localGraph = h._git->_graph; return *this; } bool commitChanges(); bool operator!(){ return !_createSuccess; } ///////////////The below operator-> it is where it does not works,////// ///////////////the retrieve of nodes with _localGraph////////////////// /// Operator-> overloading: allows access to graph members Graph* operator->() { //#ifndef NDEBUG_DG //Graph_h graph_h; NodeIterator nodeIt; Node_h node_h; //graphIt it = DynamicGraphs::_graphRep.find("COI:0351-123-4555"); //Graph_h //graph_h = Graph_h(it); propertystream_h propS; property_h p_h; propS = _localGraph.lookupLocalProp("name"); p_h = propS.next(); cout << "DEBUG@Graph::operator-> " << p_h.value() <<endl; nodeIt = _localGraph.nodeIterator("Node:0351-123-4569"); if (!nodeIt) cout << "Node version was not found!"<<endl; else //Works for both: one match and range lookup while (!nodeIt.end()){ node_h = nodeIt.next(); cout << "Found it: " << *node_h; } //#endif //NDEBUG_DG return &_localGraph; } }; Output from these strech of code: The node are not being printed because I am changing some stuff in other part of the code... but the important is that it is found the nodes in the constructor, and in the operator-> the node were not found: Constructor Output and operator-> output: DEBUG@Graph_hConstructor(it) COI:0351-123-4555 Found it: Name: Timestamp: -1080916036 Found it: Name: Timestamp: -1080916036 DEBUG@Graph::operator-> COI:0351-123-4555 Node version was not found! Code in the main which generate that: graphIt it = DynamicGraphs::_graphRep.find("COI:0351-123-4555"); graph_h = Graph_h(it); propS = graph_h->lookupLocalProp("name"); As you asked before, Here are the type of the Graph: class Graph { private: /// This variables represent the range of time that one would like to retrieve from the API. time_t _rangeBegin, _rangeEnd; /// Represents the repository of nodes in a graph nodeIndex _nodeRep; /// Represents the Node's name repository in a graph (in order to not have redundant data) uniqueNameTable _nodeNameRep; /// Represents the Node's view with index in the timestamp (in order to filter the data) nodeTimestampView _nodeTimeStampView; /// Represents the properties of a graph propIndex _localPropRep; /// Repository for the properties of the global properties related to the graph; grobalProperties _globalPropRep; The main types here that I think it could be giving me problems it would be the ones related to the nodes, here are the definitions: typedef multi_index_container< nodeMap, indexed_by< hashed_non_unique< tag<nodeName>, key_from_key< BOOST_MULTI_INDEX_MEMBER(uniqueNodeNameStruct,const string,_name), BOOST_MULTI_INDEX_MEMBER(nodeMap, const uniqueNodeNameStruct *,_uniqueName) > >, ordered_non_unique< tag<nodeTimeStamp>,BOOST_MULTI_INDEX_MEMBER(nodeMap,time_t,_timestamp) >
nodeIndex;
/// Defining an iterator for the nodeMapIndex typedef nodeIndex::iterator nodeMapIt; struct nodeMap { const uniqueNodeNameStruct* _uniqueName; time_t _timestamp; Node _node; /// Parameter nodeMap(const uniqueNodeNameStruct* name, time_t t, Node node): _uniqueName(name),_timestamp(t), _node(node) {} /// Copy Constructor nodeMap(const nodeMap &nm): _uniqueName(nm._uniqueName),_timestamp(nm._timestamp), _node(nm._node) {} //TODO: Delete this. /// For debugging friend std::ostream& operator<<(std::ostream& os,const nodeMap& nm) { os<<"Name: " << nm._uniqueName << " Timestamp: " << nm._timestamp << std::endl; return os; } }; struct uniqueNodeNameStruct { string _name; /// Parameter Constructor uniqueNodeNameStruct(const string& name):_name(name){} /// Copy Constructor uniqueNodeNameStruct(const uniqueNodeNameStruct &nodeName): _name(nodeName._name){} uniqueNodeNameStruct* operator=(uniqueNodeNameStruct& st){ _name = st._name; return &st; } friend std::ostream& operator<<(std::ostream& os,const uniqueNodeNameStruct& nodeName) { os<<"uniqueName: " << nodeName._name << endl; return os; } // TODO: Verify the necessity of change a name of the node struct change_name { change_name(const string& new_name):_new_name(new_name){} void operator()(uniqueNodeNameStruct& n) { n._name=_new_name; } private: std::string _new_name; }; }; /** * \brief uniqueNodeNames are store in this table and they are unique. * The uniqueNodeNames will be reference in this table, in order to * associate then with the different versions, in that way it's possible * to maintain just one name for all its versions. */ typedef multi_index_container< uniqueNodeNameStruct, indexed_by< hashed_unique< BOOST_MULTI_INDEX_MEMBER(uniqueNodeNameStruct,string,_name) > > > uniqueNameTable; typedef uniqueNameTable::iterator nameMapIt; I don't know how did you find that could be something in the Graph_h, perhaps you can understand that, because I don't understand what is wrong there... Looking foward for a feedback, and thanks for the help so far... best regards, Rodrigo -- ************************************************* Rodrigo Dias Ferreira Master Student of Computational Engineering Technische Universität Dresden - Germany Mobile Phone:+49 171 3158797 E-mail: rodrigodias@gmx.de ************************************************* Jetzt 1 Monat kostenlos! GMX FreeDSL - Telefonanschluss + DSL für nur 17,95 Euro/mtl.!* http://dsl.gmx.de/?ac=OM.AD.PD003K11308T4569a

Rodrigo Dias Ferreira escribió:
Hi Joaquim,
Can you please test the following? In your //Test of the commit section, rather than reuse graph_h please use a different variable to do the after-commitChanges check:
[...]
Does this make any difference?
Yes, it worked, if I instantiate another object (like graph_h2). After that I have done some more Debug to try to find the reason, but I still don't understand, because inside the constructor it finds the Nodes, when I use the same object (graph_h)... Inside the overloaded operator-> does not work anymore, even I am using the _localGraph in both stretch of codes. So if it was the assignment of the _localGraph, it should not work with the constructor also, isn't true?
What happens when you do *not* use graph_h2 is that, upon reassigning graph_h in graph_h = Graph_h(it); the previous value of graph_h is destroyed, and presumably this leaves some dangling pointer/iterator when it shouldn't. I've examined your code in search for candidates to potential dangling pointers/iterators and found something a little suspicious: Graph contains the following two member variables: uniqueNameTable _nodeNameRep; nodeIndex _nodeRep; where uniqueNameTable is a container of uniqueNodeNameStruct's and nodeIndex is a container of nodeMaps pointing to uniqueNodeNameStruct's in _nodeRep, right? If so, default copy semantics of Graph is flawed; consider: Graph g2; { Graph g1; // g1._nodeRep elements point into g1._nodeNameRep g2=g1; // g2._nodeRep elements point into g1._nodeNameRep } // g1 is destroyed // g2._nodeRep elements point to destroyed objects See what I mean? Pointers inside _nodeRep are not reassigned on copy. Does the copy constructor of Graph take this issue into account? If this leads nowhere, you can always try setting up Boost.MultiIndex safe mode: http://boost.org/libs/multi_index/doc/tutorial/debug.html#safe_mode and see if some safe mode assertion is triggered. Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
participants (2)
-
joaquin@tid.es
-
Rodrigo Dias Ferreira