Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v634] Prevent a race condition in fStreamerImpl value #17759

Merged
merged 4 commits into from
Feb 19, 2025
Merged

[v634] Prevent a race condition in fStreamerImpl value #17759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d3a71a0
[meta] Always delegate set fStreamerImpl to SetStreamerImpl
pcanal Feb 13, 2025
6493015
[meta] TClass::Property change fStreamerType only once.
pcanal Feb 13, 2025
47672c4
[meta] Set fStreamerImpl before fProperty.
pcanal Feb 13, 2025
e1ea995
[meta] Don't redo Property if no new information.
pcanal Feb 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/meta/inc/TClass.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ friend class TStreamerInfo;
void SetClassSize(Int_t sizof) { fSizeof = sizof; }
TVirtualStreamerInfo* DetermineCurrentStreamerInfo();

void SetStreamerImpl();
void SetStreamerImpl(Int_t streamerType);

void SetRuntimeProperties();

Expand Down
45 changes: 25 additions & 20 deletions core/meta/src/TClass.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6168,14 +6168,21 @@ Long_t TClass::Property() const
// Avoid asking about the class when it is still building
if (TestBit(kLoading)) return fProperty;

if (fStreamerType != kDefault && !HasInterpreterInfo()) {
// We have no interpreter information but we already set the streamer type
// so we have already been here and have no new information, then let's
// give up. See the code at this end of this routine (else branch of the
// `if (HasInterpreterInfo()` for the path we took before.
return 0;
}

// When called via TMapFile (e.g. Update()) make sure that the dictionary
// gets allocated on the heap and not in the mapped file.
TMmallocDescTemp setreset;

TClass *kl = const_cast<TClass*>(this);

kl->fStreamerType = TClass::kDefault;
kl->fStreamerImpl = &TClass::StreamerDefault;
Int_t streamerType = TClass::kDefault;

if (InheritsFrom(TObject::Class())) {
kl->SetBit(kIsTObject);
Expand All @@ -6184,8 +6191,7 @@ Long_t TClass::Property() const
Int_t delta = kl->GetBaseClassOffsetRecurse(TObject::Class());
if (delta==0) kl->SetBit(kStartWithTObject);

kl->fStreamerType = kTObject;
kl->fStreamerImpl = &TClass::StreamerTObject;
streamerType = kTObject;
}

if (HasInterpreterInfo()) {
Expand All @@ -6197,33 +6203,30 @@ Long_t TClass::Property() const
if (!const_cast<TClass*>(this)->GetClassMethodWithPrototype("Streamer","TBuffer&",kFALSE)) {

kl->SetBit(kIsForeign);
kl->fStreamerType = kForeign;
kl->fStreamerImpl = &TClass::StreamerStreamerInfo;
streamerType = kForeign;

} else if ( kl->fStreamerType == TClass::kDefault ) {
} else if (streamerType == TClass::kDefault) {
if (kl->fConvStreamerFunc) {
kl->fStreamerType = kInstrumented;
kl->fStreamerImpl = &TClass::ConvStreamerInstrumented;
streamerType = kInstrumented;
} else if (kl->fStreamerFunc) {
kl->fStreamerType = kInstrumented;
kl->fStreamerImpl = &TClass::StreamerInstrumented;
streamerType = kInstrumented;
} else {
// We have an automatic streamer using the StreamerInfo .. no need to go through the
// Streamer method function itself.
kl->fStreamerType = kInstrumented;
kl->fStreamerImpl = &TClass::StreamerStreamerInfo;
streamerType = kInstrumented;
}
}

if (fStreamer) {
kl->fStreamerType = kExternal;
kl->fStreamerImpl = &TClass::StreamerExternal;
streamerType = kExternal;
}

if (const_cast<TClass *>(this)->GetClassMethodWithPrototype("Hash", "", kTRUE)) {
kl->SetBit(kHasLocalHashMember);
}

kl->SetStreamerImpl(streamerType);

if (GetClassInfo()) {
// In the case where the TClass for one of ROOT's core class
// (eg TClonesArray for map<int,TClonesArray*>) is requested
Expand All @@ -6238,15 +6241,16 @@ Long_t TClass::Property() const
// and think all test bits have been properly set.
kl->fProperty = gCling->ClassInfo_Property(fClassInfo);
}

} else {

if (fStreamer) {
kl->fStreamerType = kExternal;
kl->fStreamerImpl = &TClass::StreamerExternal;
streamerType = kExternal;
}

kl->fStreamerType |= kEmulatedStreamer;
kl->SetStreamerImpl();
streamerType |= kEmulatedStreamer;

kl->SetStreamerImpl(streamerType);
// fProperty was *not* set so that it can be forced to be recalculated
// next time.
return 0;
Expand Down Expand Up @@ -6281,8 +6285,9 @@ void TClass::SetRuntimeProperties()
/// Internal routine to set fStreamerImpl based on the value of
/// fStreamerType.

void TClass::SetStreamerImpl()
void TClass::SetStreamerImpl(Int_t StreamerType)
{
fStreamerType = StreamerType;
switch (fStreamerType) {
case kTObject: fStreamerImpl = &TClass::StreamerTObject; break;
case kForeign: fStreamerImpl = &TClass::StreamerStreamerInfo; break;
Expand Down
3 changes: 1 addition & 2 deletions core/meta/src/TProtoClass.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,6 @@ Bool_t TProtoClass::FillTClass(TClass* cl) {
cl->fCanSplit = fCanSplit;
cl->fProperty = fProperty;
cl->fClassProperty = fClassProperty;
cl->fStreamerType = fStreamerType;

// Update pointers to TClass
if (cl->fBase.load()) {
Expand Down Expand Up @@ -405,7 +404,7 @@ Bool_t TProtoClass::FillTClass(TClass* cl) {
cl->fRealData = new TList(); // FIXME: this should really become a THashList!
}

cl->SetStreamerImpl();
cl->SetStreamerImpl(fStreamerType);

// set to zero in order not to delete when protoclass is deleted
fBase = nullptr;
Expand Down
Loading