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.

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

fix: Make event descriptor building and accessing thread safe #10445

Merged
merged 2 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,14 @@ public EventDescriptor getDescriptor() {
/**
* Build the descriptor of this event. This cannot be done when the event is first instantiated, it needs to be
* hashed before the descriptor can be built.
*
* @throws IllegalStateException if the descriptor has already been built
*/
public void buildDescriptor() {
if (descriptor != null) {
throw new IllegalStateException("Descriptor has already been built");
}

this.descriptor = hashedData.createEventDescriptor();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import static com.swirlds.logging.legacy.LogMarker.EXCEPTION;
import static com.swirlds.platform.consensus.GraphGenerations.FIRST_GENERATION;
import static com.swirlds.platform.event.creation.tipset.TipsetAdvancementWeight.ZERO_ADVANCEMENT_WEIGHT;
import static com.swirlds.platform.event.creation.tipset.TipsetUtils.buildDescriptor;
import static com.swirlds.platform.event.creation.tipset.TipsetUtils.getParentDescriptors;
import static com.swirlds.platform.system.events.EventConstants.CREATOR_ID_UNDEFINED;
import static com.swirlds.platform.system.events.EventConstants.GENERATION_UNDEFINED;
Expand Down Expand Up @@ -168,7 +167,7 @@ public void registerEvent(@NonNull final GossipEvent event) {
if (lastSelfEvent == null || lastSelfEvent.getGeneration() < event.getGeneration()) {
// Normally we will ingest self events before we get to this point, but it's possible
// to learn of self events for the first time here if we are loading from a restart or reconnect.
lastSelfEvent = buildDescriptor(event);
lastSelfEvent = event.getDescriptor();
lastSelfEventCreationTime = event.getHashedData().getTimeCreated();
lastSelfEventTransactionCount = event.getHashedData().getTransactions() == null
? 0
Expand All @@ -181,7 +180,7 @@ public void registerEvent(@NonNull final GossipEvent event) {
}
}

final EventDescriptor descriptor = buildDescriptor(event);
final EventDescriptor descriptor = event.getDescriptor();
final List<EventDescriptor> parentDescriptors = getParentDescriptors(event);

tipsetTracker.addEvent(descriptor, parentDescriptors);
Expand Down Expand Up @@ -376,7 +375,7 @@ private GossipEvent buildAndProcessEvent(@Nullable final EventDescriptor otherPa

final GossipEvent event = assembleEventObject(otherParent);

final EventDescriptor descriptor = buildDescriptor(event);
final EventDescriptor descriptor = event.getDescriptor();
tipsetTracker.addEvent(descriptor, parentDescriptors);
final TipsetAdvancementWeight advancementWeight =
tipsetWeightCalculator.addEventAndGetAdvancementWeight(descriptor);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,6 @@ public static EventDescriptor buildDescriptor(@NonNull final EventImpl event) {
return event.getHashedData().createEventDescriptor();
}

/**
* Build a descriptor from a GossipEvent.
*
* @param event the event
* @return the descriptor
*/
public static EventDescriptor buildDescriptor(@NonNull final GossipEvent event) {
event.buildDescriptor();
return event.getDescriptor();
}

/**
* Get the descriptors of an event's parents.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,9 +232,6 @@ public void awaitPrehandleCompletion() {
*/
private void setDefaultValues() {
runningHash = new RunningHash();
if (baseEvent.getHashedData().getHash() != null) {
baseEvent.buildDescriptor();
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,10 @@ public void generateAndFeed(final int numEvents) {
.limit(numEvents)
.collect(Collectors.toList());
Collections.shuffle(generatedList, r);
generatedList.forEach(intake::addUnlinkedEvent);
generatedList.forEach(event -> {
event.buildDescriptor();
intake.addUnlinkedEvent(event);
});
}

public List<EventImpl> getConsensusEvents() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ void eventGenerator() {
final OrphanBufferTester orphanBuffer = new OrphanBufferTester(pf ->
new OrphanBufferingLinker(consensusConfig, pf, GENERATIONS_STORED, mock(IntakeEventCounter.class)));
for (final GossipEvent event : generatedList) {
event.buildDescriptor();
orphanBuffer.linkEvent(event);
while (orphanBuffer.hasLinkedEvents()) {
returnedList.add(orphanBuffer.pollLinkedEvent().getBaseEvent());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import com.swirlds.platform.system.address.AddressBook;
import com.swirlds.platform.system.events.BaseEventHashedData;
import com.swirlds.platform.system.events.BaseEventUnhashedData;
import com.swirlds.platform.system.events.ConsensusData;
import com.swirlds.platform.system.events.EventConstants;
import com.swirlds.platform.system.events.EventDescriptor;
import com.swirlds.platform.system.transaction.ConsensusTransactionImpl;
Expand Down Expand Up @@ -244,8 +245,7 @@ private EventImpl linkEvent(
final EventImpl selfParent = events.get(event.getHashedData().getSelfParentHash());
final EventImpl otherParent = events.get(event.getHashedData().getOtherParentHash());

final EventImpl eventImpl =
new EventImpl(event.getHashedData(), event.getUnhashedData(), selfParent, otherParent);
final EventImpl eventImpl = new EventImpl(event, new ConsensusData(), selfParent, otherParent);
events.put(event.getHashedData().getHash(), eventImpl);

return eventImpl;
Expand Down
Loading