Merge lp:~zorba-coders/zorba/xqxq-memory-smash into lp:zorba/xqxq-module
- xqxq-memory-smash
- Merge into xqxq-module
Proposed by
Chris Hillery
Status: | Merged |
---|---|
Approved by: | Juan Zacarias |
Approved revision: | 42 |
Merged at revision: | 41 |
Proposed branch: | lp:~zorba-coders/zorba/xqxq-memory-smash |
Merge into: | lp:zorba/xqxq-module |
Diff against target: |
313 lines (+130/-43) 7 files modified
src/xqxq.xq.src/xqxq.cpp (+52/-24) src/xqxq.xq.src/xqxq.h (+23/-19) test/ExpQueryResults/xqxq/url-schema-resolver3.xml.res (+2/-0) test/Queries/xqxq/error-in-query.spec (+2/-0) test/Queries/xqxq/error-in-query.xq (+21/-0) test/Queries/xqxq/test.xml (+1/-0) test/Queries/xqxq/url-schema-resolver3.xq (+29/-0) |
To merge this branch: | bzr merge lp:~zorba-coders/zorba/xqxq-memory-smash |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juan Zacarias | Approve | ||
Chris Hillery | Approve | ||
Review via email: mp+130657@code.launchpad.net |
Commit message
Fix several memory errors having to do with freeing URIMappers / URLResolvers.
Description of the change
To post a comment you must log in.
Revision history for this message
Chris Hillery (ceejatec) : | # |
review:
Approve
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote : | # |
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote : | # |
Validation queue job xqxq-memory-
All tests succeeded!
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote : | # |
Voting does not meet specified criteria. Required: Approve > 1, Disapprove < 1, Needs Fixing < 1, Pending < 1. Got: 1 Approve, 1 Pending.
Revision history for this message
Juan Zacarias (juan457) : | # |
review:
Approve
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote : | # |
Validation queue starting for merge proposal.
Log at: http://
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote : | # |
Validation queue job xqxq-memory-
All tests succeeded!
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'src/xqxq.xq.src/xqxq.cpp' | |||
2 | --- src/xqxq.xq.src/xqxq.cpp 2012-10-19 01:22:47 +0000 | |||
3 | +++ src/xqxq.xq.src/xqxq.cpp 2012-10-20 02:20:24 +0000 | |||
4 | @@ -179,20 +179,39 @@ | |||
5 | 179 | String errDescription(aErrorMessage); | 179 | String errDescription(aErrorMessage); |
6 | 180 | throw USER_EXCEPTION(errQName, errDescription); | 180 | throw USER_EXCEPTION(errQName, errDescription); |
7 | 181 | } | 181 | } |
12 | 182 | 182 | ||
13 | 183 | /******************************************************************************************* | 183 | /******************************************************************************************* |
14 | 184 | *******************************************************************************************/ | 184 | *******************************************************************************************/ |
15 | 185 | 185 | ||
16 | 186 | QueryData::QueryData(XQuery_t aQuery, URIMapper *aMapper, URLResolver *aResolver) | ||
17 | 187 | : theQuery(aQuery), | ||
18 | 188 | theURIMapper(aMapper), | ||
19 | 189 | theURLResolver(aResolver) | ||
20 | 190 | { | ||
21 | 191 | } | ||
22 | 192 | |||
23 | 193 | QueryData::~QueryData() | ||
24 | 194 | { | ||
25 | 195 | theQuery->close(); | ||
26 | 196 | delete theURIMapper; | ||
27 | 197 | delete theURLResolver; | ||
28 | 198 | } | ||
29 | 199 | |||
30 | 200 | /******************************************************************************************* | ||
31 | 201 | *******************************************************************************************/ | ||
32 | 202 | |||
33 | 186 | QueryMap::QueryMap() | 203 | QueryMap::QueryMap() |
34 | 187 | { | 204 | { |
35 | 188 | QueryMap::queryMap = new QueryMap_t(); | 205 | QueryMap::queryMap = new QueryMap_t(); |
36 | 189 | } | 206 | } |
37 | 190 | 207 | ||
38 | 191 | bool | 208 | bool |
40 | 192 | QueryMap::storeQuery(const String& aKeyName, XQuery_t aQuery) | 209 | QueryMap::storeQuery(const String& aKeyName, XQuery_t aQuery, |
41 | 210 | URIMapper* aMapper, URLResolver* aResolver) | ||
42 | 193 | { | 211 | { |
43 | 212 | QueryData_t lQueryData(new QueryData(aQuery, aMapper, aResolver)); | ||
44 | 194 | std::pair<QueryMap_t::iterator,bool> ret; | 213 | std::pair<QueryMap_t::iterator,bool> ret; |
46 | 195 | ret = queryMap->insert(std::pair<String, XQuery_t>(aKeyName, aQuery)); | 214 | ret = queryMap->insert(std::pair<String, QueryData_t>(aKeyName, lQueryData)); |
47 | 196 | return ret.second; | 215 | return ret.second; |
48 | 197 | } | 216 | } |
49 | 198 | 217 | ||
50 | @@ -204,7 +223,7 @@ | |||
51 | 204 | if(lIter == queryMap->end()) | 223 | if(lIter == queryMap->end()) |
52 | 205 | return NULL; | 224 | return NULL; |
53 | 206 | 225 | ||
55 | 207 | XQuery_t lQuery = lIter->second; | 226 | XQuery_t lQuery = lIter->second->getQuery(); |
56 | 208 | 227 | ||
57 | 209 | return lQuery; | 228 | return lQuery; |
58 | 210 | } | 229 | } |
59 | @@ -216,14 +235,27 @@ | |||
60 | 216 | 235 | ||
61 | 217 | if(lIter == queryMap->end()) | 236 | if(lIter == queryMap->end()) |
62 | 218 | return false; | 237 | return false; |
63 | 219 | |||
64 | 220 | lIter->second->close(); | ||
65 | 221 | 238 | ||
66 | 222 | queryMap->erase(lIter); | 239 | queryMap->erase(lIter); |
67 | 223 | |||
68 | 224 | return true; | 240 | return true; |
69 | 225 | } | 241 | } |
70 | 226 | 242 | ||
71 | 243 | void | ||
72 | 244 | QueryMap::destroy() throw() | ||
73 | 245 | { | ||
74 | 246 | if(queryMap) | ||
75 | 247 | { | ||
76 | 248 | for (QueryMap_t::const_iterator lIter = queryMap->begin(); | ||
77 | 249 | lIter != queryMap->end(); ++lIter) | ||
78 | 250 | { | ||
79 | 251 | deleteQuery(lIter->first); | ||
80 | 252 | } | ||
81 | 253 | queryMap->clear(); | ||
82 | 254 | delete queryMap; | ||
83 | 255 | } | ||
84 | 256 | delete this; | ||
85 | 257 | } | ||
86 | 258 | |||
87 | 227 | /******************************************************************************************* | 259 | /******************************************************************************************* |
88 | 228 | *******************************************************************************************/ | 260 | *******************************************************************************************/ |
89 | 229 | static void streamReleaser(std::istream* aStream) | 261 | static void streamReleaser(std::istream* aStream) |
90 | @@ -344,7 +376,6 @@ | |||
91 | 344 | { | 376 | { |
92 | 345 | DynamicContext* lDynCtx = const_cast<DynamicContext*>(aDctx); | 377 | DynamicContext* lDynCtx = const_cast<DynamicContext*>(aDctx); |
93 | 346 | StaticContext_t lSctxChild = aSctx->createChildContext(); | 378 | StaticContext_t lSctxChild = aSctx->createChildContext(); |
94 | 347 | StaticContext_t lMapperSctx = aSctx->createChildContext(); | ||
95 | 348 | 379 | ||
96 | 349 | QueryMap* lQueryMap; | 380 | QueryMap* lQueryMap; |
97 | 350 | if(!(lQueryMap = dynamic_cast<QueryMap*>(lDynCtx->getExternalFunctionParameter("xqxqQueryMap")))) | 381 | if(!(lQueryMap = dynamic_cast<QueryMap*>(lDynCtx->getExternalFunctionParameter("xqxqQueryMap")))) |
98 | @@ -360,16 +391,16 @@ | |||
99 | 360 | XQuery_t lQuery; | 391 | XQuery_t lQuery; |
100 | 361 | 392 | ||
101 | 362 | StaticContext_t ltempSctx = lZorba->createStaticContext(); | 393 | StaticContext_t ltempSctx = lZorba->createStaticContext(); |
104 | 363 | XQXQURLResolver* lResolver = NULL; | 394 | std::auto_ptr<XQXQURLResolver> lResolver; |
105 | 364 | XQXQURIMapper* lMapper = NULL; | 395 | std::auto_ptr<XQXQURIMapper> lMapper; |
106 | 365 | 396 | ||
107 | 366 | if ( aArgs.size() > 2 ) | 397 | if ( aArgs.size() > 2 ) |
108 | 367 | { | 398 | { |
109 | 368 | Item lMapperFunctionItem = getItemArgument(aArgs, 2); | 399 | Item lMapperFunctionItem = getItemArgument(aArgs, 2); |
110 | 369 | if (!lMapperFunctionItem.isNull()) | 400 | if (!lMapperFunctionItem.isNull()) |
111 | 370 | { | 401 | { |
114 | 371 | lMapper = new XQXQURIMapper(lMapperFunctionItem, lSctxChild); | 402 | lMapper.reset(new XQXQURIMapper(lMapperFunctionItem, lSctxChild)); |
115 | 372 | ltempSctx->registerURIMapper(lMapper); | 403 | ltempSctx->registerURIMapper(lMapper.get()); |
116 | 373 | } | 404 | } |
117 | 374 | } | 405 | } |
118 | 375 | 406 | ||
119 | @@ -378,8 +409,8 @@ | |||
120 | 378 | Item lResolverFunctionItem = getItemArgument(aArgs, 1); | 409 | Item lResolverFunctionItem = getItemArgument(aArgs, 1); |
121 | 379 | if (!lResolverFunctionItem.isNull()) | 410 | if (!lResolverFunctionItem.isNull()) |
122 | 380 | { | 411 | { |
125 | 381 | lResolver = new XQXQURLResolver(lResolverFunctionItem, lSctxChild); | 412 | lResolver.reset(new XQXQURLResolver(lResolverFunctionItem, lSctxChild)); |
126 | 382 | ltempSctx->registerURLResolver(lResolver); | 413 | ltempSctx->registerURLResolver(lResolver.get()); |
127 | 383 | } | 414 | } |
128 | 384 | 415 | ||
129 | 385 | } | 416 | } |
130 | @@ -390,6 +421,7 @@ | |||
131 | 390 | } | 421 | } |
132 | 391 | catch (XQueryException& xe) | 422 | catch (XQueryException& xe) |
133 | 392 | { | 423 | { |
134 | 424 | lQuery = NULL; | ||
135 | 393 | std::ostringstream err; | 425 | std::ostringstream err; |
136 | 394 | err << "The query compiled using xqxq:prepare-main-module raised an error at" | 426 | err << "The query compiled using xqxq:prepare-main-module raised an error at" |
137 | 395 | << " line " << xe.source_line() << " column " << xe.source_column() << ": " << xe.what(); | 427 | << " line " << xe.source_line() << " column " << xe.source_column() << ": " << xe.what(); |
138 | @@ -399,6 +431,7 @@ | |||
139 | 399 | } | 431 | } |
140 | 400 | catch (ZorbaException& e) | 432 | catch (ZorbaException& e) |
141 | 401 | { | 433 | { |
142 | 434 | lQuery = NULL; | ||
143 | 402 | std::ostringstream err; | 435 | std::ostringstream err; |
144 | 403 | err << "The query compiled using xqxq:prepare-main-module raised an error: " | 436 | err << "The query compiled using xqxq:prepare-main-module raised an error: " |
145 | 404 | << e.what(); | 437 | << e.what(); |
146 | @@ -406,7 +439,7 @@ | |||
147 | 406 | e.diagnostic().qname().ns(), e.diagnostic().qname().localname()); | 439 | e.diagnostic().qname().ns(), e.diagnostic().qname().localname()); |
148 | 407 | throw USER_EXCEPTION(errQName, err.str()); | 440 | throw USER_EXCEPTION(errQName, err.str()); |
149 | 408 | } | 441 | } |
151 | 409 | 442 | ||
152 | 410 | uuid lUUID; | 443 | uuid lUUID; |
153 | 411 | uuid::create(&lUUID); | 444 | uuid::create(&lUUID); |
154 | 412 | 445 | ||
155 | @@ -415,13 +448,8 @@ | |||
156 | 415 | 448 | ||
157 | 416 | String lStrUUID = lStream.str(); | 449 | String lStrUUID = lStream.str(); |
158 | 417 | 450 | ||
160 | 418 | lQueryMap->storeQuery(lStrUUID, lQuery); | 451 | lQueryMap->storeQuery(lStrUUID, lQuery, lMapper.release(), lResolver.release()); |
161 | 419 | 452 | ||
162 | 420 | if (lResolver) | ||
163 | 421 | delete lResolver; | ||
164 | 422 | if (lMapper) | ||
165 | 423 | delete lMapper; | ||
166 | 424 | |||
167 | 425 | return ItemSequence_t(new SingletonItemSequence(XQXQModule::getItemFactory()->createAnyURI(lStrUUID))); | 453 | return ItemSequence_t(new SingletonItemSequence(XQXQModule::getItemFactory()->createAnyURI(lStrUUID))); |
168 | 426 | } | 454 | } |
169 | 427 | 455 | ||
170 | 428 | 456 | ||
171 | === modified file 'src/xqxq.xq.src/xqxq.h' | |||
172 | --- src/xqxq.xq.src/xqxq.h 2012-10-18 00:34:20 +0000 | |||
173 | +++ src/xqxq.xq.src/xqxq.h 2012-10-20 02:20:24 +0000 | |||
174 | @@ -49,35 +49,39 @@ | |||
175 | 49 | 49 | ||
176 | 50 | }; | 50 | }; |
177 | 51 | 51 | ||
182 | 52 | 52 | /** | |
183 | 53 | class QueryMap : public ExternalFunctionParameter{ | 53 | * @brief Bag class for objects associated with a prepared query |
184 | 54 | private: | 54 | */ |
185 | 55 | typedef std::map<String, XQuery_t> QueryMap_t; | 55 | class QueryData : public SmartObject |
186 | 56 | { | ||
187 | 57 | private: | ||
188 | 58 | XQuery_t theQuery; | ||
189 | 59 | URIMapper* theURIMapper; | ||
190 | 60 | URLResolver* theURLResolver; | ||
191 | 61 | |||
192 | 62 | public: | ||
193 | 63 | QueryData(XQuery_t aQuery, URIMapper* aMapper, URLResolver* aResolver); | ||
194 | 64 | virtual ~QueryData(); | ||
195 | 65 | XQuery_t getQuery() { return theQuery; } | ||
196 | 66 | }; | ||
197 | 67 | typedef SmartPtr<QueryData> QueryData_t; | ||
198 | 68 | |||
199 | 69 | class QueryMap : public ExternalFunctionParameter | ||
200 | 70 | { | ||
201 | 71 | private: | ||
202 | 72 | typedef std::map<String, QueryData_t> QueryMap_t; | ||
203 | 56 | QueryMap_t* queryMap; | 73 | QueryMap_t* queryMap; |
204 | 57 | 74 | ||
205 | 58 | public: | 75 | public: |
206 | 59 | QueryMap(); | 76 | QueryMap(); |
207 | 60 | bool | 77 | bool |
209 | 61 | storeQuery(const String&, XQuery_t); | 78 | storeQuery(const String&, XQuery_t, URIMapper*, URLResolver*); |
210 | 62 | XQuery_t | 79 | XQuery_t |
211 | 63 | getQuery(const String&); | 80 | getQuery(const String&); |
212 | 64 | bool | 81 | bool |
213 | 65 | deleteQuery(const String&); | 82 | deleteQuery(const String&); |
214 | 66 | virtual void | 83 | virtual void |
229 | 67 | destroy() throw() | 84 | destroy() throw(); |
216 | 68 | { | ||
217 | 69 | if(queryMap) | ||
218 | 70 | { | ||
219 | 71 | for (QueryMap_t::const_iterator lIter = queryMap->begin(); | ||
220 | 72 | lIter != queryMap->end(); ++lIter) | ||
221 | 73 | { | ||
222 | 74 | lIter->second->close(); | ||
223 | 75 | } | ||
224 | 76 | queryMap->clear(); | ||
225 | 77 | delete queryMap; | ||
226 | 78 | } | ||
227 | 79 | delete this; | ||
228 | 80 | }; | ||
230 | 81 | }; | 85 | }; |
231 | 82 | 86 | ||
232 | 83 | class XQXQFunction : public ContextualExternalFunction | 87 | class XQXQFunction : public ContextualExternalFunction |
233 | 84 | 88 | ||
234 | === added file 'test/ExpQueryResults/xqxq/url-schema-resolver3.xml.res' | |||
235 | --- test/ExpQueryResults/xqxq/url-schema-resolver3.xml.res 1970-01-01 00:00:00 +0000 | |||
236 | +++ test/ExpQueryResults/xqxq/url-schema-resolver3.xml.res 2012-10-20 02:20:24 +0000 | |||
237 | @@ -0,0 +1,2 @@ | |||
238 | 1 | <?xml version="1.0" encoding="UTF-8"?> | ||
239 | 2 | <test:test xmlns:test="http://test"><test:subtest>a</test:subtest><test:subtest2>a</test:subtest2></test:test> | ||
240 | 0 | 3 | ||
241 | === added file 'test/Queries/xqxq/error-in-query.spec' | |||
242 | --- test/Queries/xqxq/error-in-query.spec 1970-01-01 00:00:00 +0000 | |||
243 | +++ test/Queries/xqxq/error-in-query.spec 2012-10-20 02:20:24 +0000 | |||
244 | @@ -0,0 +1,2 @@ | |||
245 | 1 | Error: http://www.w3.org/2005/xqt-errors:XQST0033 | ||
246 | 2 | |||
247 | 0 | 3 | ||
248 | === added file 'test/Queries/xqxq/error-in-query.xq' | |||
249 | --- test/Queries/xqxq/error-in-query.xq 1970-01-01 00:00:00 +0000 | |||
250 | +++ test/Queries/xqxq/error-in-query.xq 2012-10-20 02:20:24 +0000 | |||
251 | @@ -0,0 +1,21 @@ | |||
252 | 1 | import module namespace xqxq = 'http://www.zorba-xquery.com/modules/xqxq'; | ||
253 | 2 | |||
254 | 3 | declare namespace op = 'http://www.zorba-xquery.com/options/features'; | ||
255 | 4 | declare namespace f = 'http://www.zorba-xquery.com/features'; | ||
256 | 5 | declare option op:enable 'f:hof'; | ||
257 | 6 | |||
258 | 7 | declare function local:url-resolver($namespace as xs:string, $entity as xs:string) { | ||
259 | 8 | switch($entity) | ||
260 | 9 | case 'schema' | ||
261 | 10 | return switch($namespace) | ||
262 | 11 | case 'http://www.w3.org/XQueryTest' return doc('/tmp/atomic.xsd') | ||
263 | 12 | default return () | ||
264 | 13 | default return () | ||
265 | 14 | }; | ||
266 | 15 | |||
267 | 16 | variable $queryID := xqxq:prepare-main-module( | ||
268 | 17 | "declare namespace foo='http://test'; | ||
269 | 18 | declare namespace foo='http://test'; | ||
270 | 19 | 1", | ||
271 | 20 | local:url-resolver#2, ()); | ||
272 | 21 | xqxq:evaluate($queryID) | ||
273 | 0 | 22 | ||
274 | === added file 'test/Queries/xqxq/test.xml' | |||
275 | --- test/Queries/xqxq/test.xml 1970-01-01 00:00:00 +0000 | |||
276 | +++ test/Queries/xqxq/test.xml 2012-10-20 02:20:24 +0000 | |||
277 | @@ -0,0 +1,1 @@ | |||
278 | 1 | <test:test xmlns:test="http://test"><test:subtest>a</test:subtest><test:subtest2>a</test:subtest2></test:test> | ||
279 | 0 | \ No newline at end of file | 2 | \ No newline at end of file |
280 | 1 | 3 | ||
281 | === added file 'test/Queries/xqxq/url-schema-resolver3.xq' | |||
282 | --- test/Queries/xqxq/url-schema-resolver3.xq 1970-01-01 00:00:00 +0000 | |||
283 | +++ test/Queries/xqxq/url-schema-resolver3.xq 2012-10-20 02:20:24 +0000 | |||
284 | @@ -0,0 +1,29 @@ | |||
285 | 1 | import module namespace xqxq = 'http://www.zorba-xquery.com/modules/xqxq'; | ||
286 | 2 | |||
287 | 3 | declare namespace resolver = 'http://www.zorba-xquery.com/modules/xqxq/url-resolver'; | ||
288 | 4 | |||
289 | 5 | declare namespace op = "http://www.zorba-xquery.com/options/features"; | ||
290 | 6 | declare namespace f = "http://www.zorba-xquery.com/features"; | ||
291 | 7 | declare option op:enable "f:hof"; | ||
292 | 8 | |||
293 | 9 | declare function resolver:url-resolver($namespace as xs:string, $entity as xs:string) { | ||
294 | 10 | if($namespace = 'http://test' and $entity = 'schema') | ||
295 | 11 | then | ||
296 | 12 | doc('test.xsd') | ||
297 | 13 | else | ||
298 | 14 | () | ||
299 | 15 | }; | ||
300 | 16 | |||
301 | 17 | variable $contextQueryID := xqxq:prepare-main-module( | ||
302 | 18 | "import schema namespace test = 'http://test'; | ||
303 | 19 | declare variable $cwd as xs:anyURI external; | ||
304 | 20 | validate { doc(resolve-uri('test.xml', $cwd)) }", | ||
305 | 21 | resolver:url-resolver#2, ()); | ||
306 | 22 | xqxq:bind-variable($contextQueryID, fn:QName("", "cwd"), resolve-uri(".")); | ||
307 | 23 | variable $contextItem := xqxq:evaluate($contextQueryID); | ||
308 | 24 | |||
309 | 25 | variable $queryID := xqxq:prepare-main-module( | ||
310 | 26 | "import schema namespace test = 'http://test'; //*:test", | ||
311 | 27 | resolver:url-resolver#2, ()); | ||
312 | 28 | xqxq:bind-context-item($queryID, $contextItem); | ||
313 | 29 | xqxq:evaluate($queryID) |
Validation queue starting for merge proposal. zorbatest. lambda. nu:8080/ remotequeue/ xqxq-memory- smash-2012- 10-20T02- 21-41.631Z/ log.html
Log at: http://