forked from cms-sw/cmsdist
-
Notifications
You must be signed in to change notification settings - Fork 0
/
0006-Fix-thread-safety-of-TGenCollectionProxy-s-iterator-.patch
392 lines (348 loc) · 19.5 KB
/
0006-Fix-thread-safety-of-TGenCollectionProxy-s-iterator-.patch
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
From f6d7496a63ab3e4a1f847e6c36791692f93b6a5f Mon Sep 17 00:00:00 2001
From: David Abdurachmanov <[email protected]>
Date: Tue, 22 Oct 2013 14:19:05 +0200
Subject: [PATCH 2/2] Fix thread safety of TGenCollectionProxy's iterator creation.
The major change is that the utility routine returned by
TVirtualCollectionProxy::GetFunctionCreateIterators
now has the prototype:
typedef void (*CreateIterators_t)(void *collection, void **begin_arena, void **end_arena, TVirtualCollectionProxy *proxy);
i.e. adding a new pointer to the virtual collection proxy so
that in the case where the routine can not hard code all
the information (for example in the case of emulated collection
proxy) it can now get the information from a the CollectionProxy
without relying on the calls to PushProxy (which we are trying
to deprecate in general as it is somewhat thread adverse).
Signed-off-by: David Abdurachmanov <[email protected]>
---
cint/reflex/inc/Reflex/Builder/CollectionProxy.h | 15 +++++++++------
core/cont/inc/TCollectionProxyInfo.h | 14 +++++++-------
core/cont/inc/TVirtualCollectionProxy.h | 4 ++--
io/io/inc/TVirtualCollectionIterators.h | 10 +++++-----
io/io/src/TGenCollectionProxy.cxx | 20 +++++---------------
io/io/src/TStreamerInfoActions.cxx | 12 ++++++------
tree/tree/src/TBranchElement.cxx | 8 ++++----
7 files changed, 38 insertions(+), 45 deletions(-)
diff --git a/cint/reflex/inc/Reflex/Builder/CollectionProxy.h b/cint/reflex/inc/Reflex/Builder/CollectionProxy.h
index 34a540e..ce1eb10 100644
--- a/cint/reflex/inc/Reflex/Builder/CollectionProxy.h
+++ b/cint/reflex/inc/Reflex/Builder/CollectionProxy.h
@@ -22,6 +22,9 @@
#define REFLEX_COLLECTIONPROXY_VERSION 3
// Forward declarations
+
+class TVirtualCollectionProxy;
+
namespace std {
template <class T, class A> class deque;
template <class T, class A> class vector;
@@ -141,7 +144,7 @@ namespace Reflex {
typedef Cont_t *PCont_t;
typedef typename Cont_t::iterator iterator;
- static void create(void *coll, void **begin_arena, void **end_arena) {
+ static void create(void *coll, void **begin_arena, void **end_arena, TVirtualCollectionProxy *) {
PCont_t c = PCont_t(coll);
new (*begin_arena) iterator(c->begin());
new (*end_arena) iterator(c->end());
@@ -182,7 +185,7 @@ namespace Reflex {
typedef Cont_t *PCont_t;
typedef typename Cont_t::iterator iterator;
- static void create(void *coll, void **begin_arena, void **end_arena) {
+ static void create(void *coll, void **begin_arena, void **end_arena, TVirtualCollectionProxy *) {
PCont_t c = PCont_t(coll);
if (c->empty()) {
*begin_arena = 0;
@@ -219,7 +222,7 @@ namespace Reflex {
typedef Cont_t *PCont_t;
typedef Cont_t::iterator iterator;
- static void create(void *coll, void **begin_arena, void **end_arena) {
+ static void create(void *coll, void **begin_arena, void **end_arena, TVirtualCollectionProxy *) {
PCont_t c = PCont_t(coll);
new (*begin_arena) iterator(c->begin());
new (*end_arena) iterator(c->end());
@@ -255,7 +258,7 @@ namespace Reflex {
typedef Cont_t *PCont_t;
typedef typename Cont_t::iterator iterator;
- static void create(void *coll, void **begin_arena, void **end_arena) {
+ static void create(void *coll, void **begin_arena, void **end_arena, TVirtualCollectionProxy *) {
PCont_t c = PCont_t(coll);
*begin_arena = new iterator(c->begin());
*end_arena = new iterator(c->end());
@@ -563,7 +566,7 @@ namespace Reflex {
void* (*create_env)();
// Set of function of direct iteration of the collections.
- void (*fCreateIterators)(void *collection, void **begin_arena, void **end_arena);
+ void (*fCreateIterators)(void *collection, void **begin_arena, void **end_arena, TVirtualCollectionProxy *proxy);
// begin_arena and end_arena should contain the location of memory arena of size fgIteratorSize.
// If the collection iterator are of that size or less, the iterators will be constructed in place in those location (new with placement)
// Otherwise the iterators will be allocated via a regular new and their address returned by modifying the value of begin_arena and end_arena.
@@ -912,7 +915,7 @@ namespace Reflex {
// In the other iterator we store the index
// and the value.
- static void create(void *coll, void **begin_arena, void **end_arena) {
+ static void create(void *coll, void **begin_arena, void **end_arena, TVirtualCollectionProxy *) {
iterator *begin = new (*begin_arena) iterator;
begin->first.fIndex = 0;
begin->second = false;
diff --git a/core/cont/inc/TCollectionProxyInfo.h b/core/cont/inc/TCollectionProxyInfo.h
index d2ac21d..d56b758 100644
--- a/core/cont/inc/TCollectionProxyInfo.h
+++ b/core/cont/inc/TCollectionProxyInfo.h
@@ -78,7 +78,7 @@ namespace ROOT {
typedef Cont_t *PCont_t;
typedef typename Cont_t::iterator iterator;
- static void create(void *coll, void **begin_arena, void **end_arena) {
+ static void create(void *coll, void **begin_arena, void **end_arena, TVirtualCollectionProxy*) {
PCont_t c = PCont_t(coll);
new (*begin_arena) iterator(c->begin());
new (*end_arena) iterator(c->end());
@@ -119,7 +119,7 @@ namespace ROOT {
typedef Cont_t *PCont_t;
typedef typename Cont_t::iterator iterator;
- static void create(void *coll, void **begin_arena, void **end_arena) {
+ static void create(void *coll, void **begin_arena, void **end_arena, TVirtualCollectionProxy*) {
PCont_t c = PCont_t(coll);
if (c->empty()) {
*begin_arena = 0;
@@ -155,7 +155,7 @@ namespace ROOT {
typedef Cont_t *PCont_t;
typedef typename Cont_t::iterator iterator;
- static void create(void *coll, void **begin_arena, void **end_arena) {
+ static void create(void *coll, void **begin_arena, void **end_arena, TVirtualCollectionProxy*) {
PCont_t c = PCont_t(coll);
*begin_arena = new iterator(c->begin());
*end_arena = new iterator(c->end());
@@ -442,7 +442,7 @@ namespace ROOT {
void* (*fCreateEnv)();
// Set of function of direct iteration of the collections.
- void (*fCreateIterators)(void *collection, void **begin_arena, void **end_arena);
+ void (*fCreateIterators)(void *collection, void **begin_arena, void **end_arena, TVirtualCollectionProxy *proxy);
// begin_arena and end_arena should contain the location of memory arena of size fgIteratorSize.
// If the collection iterator are of that size or less, the iterators will be constructed in place in those location (new with placement)
// Otherwise the iterators will be allocated via a regular new and their address returned by modifying the value of begin_arena and end_arena.
@@ -478,7 +478,7 @@ namespace ROOT {
void* (*feed_func)(void*,void*,size_t),
void* (*collect_func)(void*,void*),
void* (*create_env)(),
- void (*getIterators)(void *collection, void **begin_arena, void **end_arena) = 0,
+ void (*getIterators)(void *collection, void **begin_arena, void **end_arena, TVirtualCollectionProxy *proxy) = 0,
void* (*copyIterator)(void *dest, const void *source) = 0,
void* (*next)(void *iter, const void *end) = 0,
void (*deleteSingleIterator)(void *iter) = 0,
@@ -617,7 +617,7 @@ namespace ROOT {
typedef Cont_t *PCont_t;
typedef Cont_t::iterator iterator;
- static void create(void *coll, void **begin_arena, void **end_arena) {
+ static void create(void *coll, void **begin_arena, void **end_arena, TVirtualCollectionProxy*) {
PCont_t c = PCont_t(coll);
new (*begin_arena) iterator(c->begin());
new (*end_arena) iterator(c->end());
@@ -759,7 +759,7 @@ namespace ROOT {
// In the other iterator we store the index
// and the value.
- static void create(void *coll, void **begin_arena, void **end_arena) {
+ static void create(void *coll, void **begin_arena, void **end_arena, TVirtualCollectionProxy*) {
iterator *begin = new (*begin_arena) iterator;
begin->first.fIndex = 0;
begin->second = false;
diff --git a/core/cont/inc/TVirtualCollectionProxy.h b/core/cont/inc/TVirtualCollectionProxy.h
index f761e5a..8a495b2 100644
--- a/core/cont/inc/TVirtualCollectionProxy.h
+++ b/core/cont/inc/TVirtualCollectionProxy.h
@@ -1,4 +1,4 @@
-// @(#)root/cont:$Id$
+// @(#)root/cont:$Id: f761e5ab393bb556962776de666bc9dd0b6e738b $
// Author: Philippe Canal 20/08/2003
/*************************************************************************
@@ -159,7 +159,7 @@ public:
// Set of functions to iterate easily throught the collection
static const Int_t fgIteratorArenaSize = 16; // greater than sizeof(void*) + sizeof(UInt_t)
- typedef void (*CreateIterators_t)(void *collection, void **begin_arena, void **end_arena);
+ typedef void (*CreateIterators_t)(void *collection, void **begin_arena, void **end_arena, TVirtualCollectionProxy *proxy);
virtual CreateIterators_t GetFunctionCreateIterators(Bool_t read = kTRUE) = 0;
// begin_arena and end_arena should contain the location of a memory arena of size fgIteratorSize.
// If the collection iterator are of that size or less, the iterators will be constructed in place in those location (new with placement)
diff --git a/io/io/inc/TVirtualCollectionIterators.h b/io/io/inc/TVirtualCollectionIterators.h
index e00edea..8f5fc7f 100644
--- a/io/io/inc/TVirtualCollectionIterators.h
+++ b/io/io/inc/TVirtualCollectionIterators.h
@@ -1,4 +1,4 @@
-// @(#)root/cont:$Id$
+// @(#)root/cont:$Id: e00edea30f17233d3f97a85eba14e20c201eb980 $
// Author: Philippe Canal 20/08/2010
/*************************************************************************
@@ -63,11 +63,11 @@ public:
{
}
- inline void CreateIterators(void *collection)
+ inline void CreateIterators(void *collection, TVirtualCollectionProxy *proxy)
{
// Initialize the fBegin and fEnd iterators.
- fCreateIterators(collection, &fBegin, &fEnd);
+ fCreateIterators(collection, &fBegin, &fEnd, proxy);
}
inline ~TVirtualCollectionIterators()
@@ -143,13 +143,13 @@ public:
}
}
- inline void CreateIterators(void *collection)
+ inline void CreateIterators(void *collection, TVirtualCollectionProxy *proxy)
{
// Initialize the fBegin and fEnd iterators.
fBegin = &(fRawBeginBuffer[0]);
fEnd = &(fRawEndBuffer[0]);
- fCreateIterators(collection, &fBegin, &fEnd);
+ fCreateIterators(collection, &fBegin, &fEnd, proxy);
if (fBegin != &(fRawBeginBuffer[0])) {
// The iterator where too large to buffer in the buffer
fAllocated = kTRUE;
diff --git a/io/io/src/TGenCollectionProxy.cxx b/io/io/src/TGenCollectionProxy.cxx
index 44009e4..451bc08 100644
--- a/io/io/src/TGenCollectionProxy.cxx
+++ b/io/io/src/TGenCollectionProxy.cxx
@@ -40,8 +40,6 @@
#define MESSAGE(which,text)
-std::vector<TVirtualCollectionProxy*> gSlowIterator__Proxy;
-
//////////////////////////////////////////////////////////////////////////
// //
// class TGenVectorProxy
@@ -1121,8 +1119,6 @@ void TGenCollectionProxy::PushProxy(void *objstart)
{
// Add an object.
- gSlowIterator__Proxy.push_back(this);
-
if ( !fValue ) Initialize(kFALSE);
if ( !fProxyList.empty() ) {
EnvironBase_t* back = fProxyList.back();
@@ -1158,8 +1154,6 @@ void TGenCollectionProxy::PopProxy()
{
// Remove the last object.
- gSlowIterator__Proxy.pop_back();
-
if ( !fProxyList.empty() ) {
EnvironBase_t* e = fProxyList.back();
if ( --e->fRefCount <= 0 ) {
@@ -1277,10 +1271,10 @@ struct TGenCollectionProxy__SlowIterator {
};
//______________________________________________________________________________
-void TGenCollectionProxy__SlowCreateIterators(void * /* collection */, void **begin_arena, void **end_arena)
+void TGenCollectionProxy__SlowCreateIterators(void * /* collection */, void **begin_arena, void **end_arena, TVirtualCollectionProxy *proxy)
{
- new (*begin_arena) TGenCollectionProxy__SlowIterator(gSlowIterator__Proxy.back());
- *(UInt_t*)*end_arena = gSlowIterator__Proxy.back()->Size();
+ new (*begin_arena) TGenCollectionProxy__SlowIterator(proxy);
+ *(UInt_t*)*end_arena = proxy->Size();
}
//______________________________________________________________________________
@@ -1317,7 +1311,7 @@ void TGenCollectionProxy__SlowDeleteTwoIterators(void *, void *)
//______________________________________________________________________________
-void TGenCollectionProxy__VectorCreateIterators(void *obj, void **begin_arena, void **end_arena)
+void TGenCollectionProxy__VectorCreateIterators(void *obj, void **begin_arena, void **end_arena, TVirtualCollectionProxy*)
{
// We can safely assume that the std::vector layout does not really depend on
// the content!
@@ -1335,10 +1329,6 @@ void TGenCollectionProxy__VectorCreateIterators(void *obj, void **begin_arena, v
*end_arena = &(*vec->end());
#endif
- // The following is a safer way but require the caller to have called TPushPop
- // TVirtualCollectionProxy *proxy = gSlowIterator__Proxy.back();
- // void *good_begin_arena = proxy->At(0);
- // void *good_end_arena = ((char*)proxy->At(0)) + proxy->Size() * proxy->GetIncrement();
}
//______________________________________________________________________________
@@ -1371,7 +1361,7 @@ void TGenCollectionProxy__VectorDeleteTwoIterators(void *, void *)
//______________________________________________________________________________
-void TGenCollectionProxy__StagingCreateIterators(void *obj, void **begin_arena, void **end_arena)
+void TGenCollectionProxy__StagingCreateIterators(void *obj, void **begin_arena, void **end_arena, TVirtualCollectionProxy *)
{
TGenCollectionProxy::TStaging * s = (TGenCollectionProxy::TStaging*)obj;
*begin_arena = s->GetContent();
diff --git a/io/io/src/TStreamerInfoActions.cxx b/io/io/src/TStreamerInfoActions.cxx
index f64f861..15cc5e8 100644
--- a/io/io/src/TStreamerInfoActions.cxx
+++ b/io/io/src/TStreamerInfoActions.cxx
@@ -490,7 +490,7 @@ namespace TStreamerInfoActions
char endbuf[TVirtualCollectionProxy::fgIteratorArenaSize];
void *begin = &(startbuf[0]);
void *end = &(endbuf[0]);
- config->fCreateIterators(alternative, &begin, &end );
+ config->fCreateIterators(alternative, &begin, &end, oldProxy);
// We can not get here with a split vector of pointer, so we can indeed assume
// that actions->fConfiguration != null.
buf.ApplySequence(*actions, begin, end);
@@ -565,7 +565,7 @@ namespace TStreamerInfoActions
char endbuf[TVirtualCollectionProxy::fgIteratorArenaSize];
void *begin = &(startbuf[0]);
void *end = &(endbuf[0]);
- config->fCreateIterators(alternative, &begin, &end );
+ config->fCreateIterators(alternative, &begin, &end, oldProxy);
// We can not get here with a split vector of pointer, so we can indeed assume
// that actions->fConfiguration != null.
buf.ApplySequence(*actions, begin, end);
@@ -643,7 +643,7 @@ namespace TStreamerInfoActions
char endbuf[TVirtualCollectionProxy::fgIteratorArenaSize];
void *begin = &(startbuf[0]);
void *end = &(endbuf[0]);
- config->fCreateIterators( alternative, &begin, &end );
+ config->fCreateIterators( alternative, &begin, &end, newProxy);
// We can not get here with a split vector of pointer, so we can indeed assume
// that actions->fConfiguration != null.
buf.ApplySequence(*actions, begin, end);
@@ -692,7 +692,7 @@ namespace TStreamerInfoActions
char endbuf[TVirtualCollectionProxy::fgIteratorArenaSize];
void *begin = &(startbuf[0]);
void *end = &(endbuf[0]);
- config->fCreateIterators( alternative, &begin, &end );
+ config->fCreateIterators( alternative, &begin, &end, newProxy);
// We can not get here with a split vector of pointer, so we can indeed assume
// that actions->fConfiguration != null.
buf.ApplySequence(*actions, begin, end);
@@ -1516,7 +1516,7 @@ namespace TStreamerInfoActions
char endbuf[TVirtualCollectionProxy::fgIteratorArenaSize];
void *begin = &(startbuf[0]);
void *end = &(endbuf[0]);
- config->fCreateIterators(alternative, &begin, &end );
+ config->fCreateIterators(alternative, &begin, &end, newProxy);
// We can not get here with a split vector of pointer, so we can indeed assume
// that actions->fConfiguration != null.
@@ -1871,7 +1871,7 @@ namespace TStreamerInfoActions
char endbuf[TVirtualCollectionProxy::fgIteratorArenaSize];
void *begin = &(startbuf[0]);
void *end = &(endbuf[0]);
- config->fCreateIterators(alternative, &begin, &end );
+ config->fCreateIterators(alternative, &begin, &end, newProxy);
// We can not get here with a split vector of pointer, so we can indeed assume
// that actions->fConfiguration != null.
diff --git a/tree/tree/src/TBranchElement.cxx b/tree/tree/src/TBranchElement.cxx
index 2615eca..7de358b 100644
--- a/tree/tree/src/TBranchElement.cxx
+++ b/tree/tree/src/TBranchElement.cxx
@@ -1409,7 +1409,7 @@ void TBranchElement::FillLeavesCollection(TBuffer& b)
b << n;
if(fSTLtype != TClassEdit::kVector && proxy->HasPointers() && fSplitLevel > TTree::kSplitCollectionOfPointers ) {
- fPtrIterators->CreateIterators(fObject);
+ fPtrIterators->CreateIterators(fObject, proxy);
} else {
//NOTE: this does not work for not vectors since the CreateIterators expects a TGenCollectionProxy::TStaging as its argument!
//NOTE: and those not work in general yet, since the TStaging object is neither created nor passed.
@@ -1417,7 +1417,7 @@ void TBranchElement::FillLeavesCollection(TBuffer& b)
if (proxy->GetProperties() & TVirtualCollectionProxy::kIsAssociative) {
// do nothing for now ...
} else {
- fIterators->CreateIterators(fObject);
+ fIterators->CreateIterators(fObject, proxy);
}
}
@@ -3708,9 +3708,9 @@ void TBranchElement::ReadLeavesCollection(TBuffer& b)
TVirtualCollectionProxy::TPushPop helper(proxy, fObject);
void* alternate = proxy->Allocate(fNdata, true);
if(fSTLtype != TClassEdit::kVector && proxy->HasPointers() && fSplitLevel > TTree::kSplitCollectionOfPointers ) {
- fPtrIterators->CreateIterators(alternate);
+ fPtrIterators->CreateIterators(alternate, proxy);
} else {
- fIterators->CreateIterators(alternate);
+ fIterators->CreateIterators(alternate, proxy);
}
Int_t nbranches = fBranches.GetEntriesFast();
--
1.7.4.1