From aff5fcec670cc9a33aae7091484fe3facce0ec88 Mon Sep 17 00:00:00 2001 From: Austin Littley <102969658+alittley@users.noreply.github.com> Date: Tue, 12 Dec 2023 16:11:29 -0500 Subject: [PATCH] fix: Make event descriptor building and accessing thread safe (#10445) Signed-off-by: Austin Littley --- .../java/com/swirlds/platform/event/GossipEvent.java | 6 ++++++ .../event/creation/tipset/TipsetEventCreator.java | 7 +++---- .../platform/event/creation/tipset/TipsetUtils.java | 11 ----------- .../java/com/swirlds/platform/internal/EventImpl.java | 3 --- .../test/event/intake/OrphanEventsIntakeTest.java | 5 ++++- .../test/event/linking/OrphanBufferingLinkerTest.java | 1 + .../test/event/tipset/TipsetEventCreatorTests.java | 4 ++-- 7 files changed, 16 insertions(+), 21 deletions(-) diff --git a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/GossipEvent.java b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/GossipEvent.java index 8141d8138a27..7836063bb275 100644 --- a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/GossipEvent.java +++ b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/GossipEvent.java @@ -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(); } diff --git a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/creation/tipset/TipsetEventCreator.java b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/creation/tipset/TipsetEventCreator.java index b06c1329be11..e1578bff0f4d 100644 --- a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/creation/tipset/TipsetEventCreator.java +++ b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/creation/tipset/TipsetEventCreator.java @@ -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; @@ -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 @@ -181,7 +180,7 @@ public void registerEvent(@NonNull final GossipEvent event) { } } - final EventDescriptor descriptor = buildDescriptor(event); + final EventDescriptor descriptor = event.getDescriptor(); final List parentDescriptors = getParentDescriptors(event); tipsetTracker.addEvent(descriptor, parentDescriptors); @@ -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); diff --git a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/creation/tipset/TipsetUtils.java b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/creation/tipset/TipsetUtils.java index 05581daab2e7..d013d1ac6fe6 100644 --- a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/creation/tipset/TipsetUtils.java +++ b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/creation/tipset/TipsetUtils.java @@ -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. * diff --git a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/internal/EventImpl.java b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/internal/EventImpl.java index 22de6e8b1e14..f07d782a6e6b 100644 --- a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/internal/EventImpl.java +++ b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/internal/EventImpl.java @@ -232,9 +232,6 @@ public void awaitPrehandleCompletion() { */ private void setDefaultValues() { runningHash = new RunningHash(); - if (baseEvent.getHashedData().getHash() != null) { - baseEvent.buildDescriptor(); - } } /** diff --git a/platform-sdk/swirlds-unit-tests/core/swirlds-platform-test/src/test/java/com/swirlds/platform/test/event/intake/OrphanEventsIntakeTest.java b/platform-sdk/swirlds-unit-tests/core/swirlds-platform-test/src/test/java/com/swirlds/platform/test/event/intake/OrphanEventsIntakeTest.java index 492120791b93..fc71244e00e2 100644 --- a/platform-sdk/swirlds-unit-tests/core/swirlds-platform-test/src/test/java/com/swirlds/platform/test/event/intake/OrphanEventsIntakeTest.java +++ b/platform-sdk/swirlds-unit-tests/core/swirlds-platform-test/src/test/java/com/swirlds/platform/test/event/intake/OrphanEventsIntakeTest.java @@ -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 getConsensusEvents() { diff --git a/platform-sdk/swirlds-unit-tests/core/swirlds-platform-test/src/test/java/com/swirlds/platform/test/event/linking/OrphanBufferingLinkerTest.java b/platform-sdk/swirlds-unit-tests/core/swirlds-platform-test/src/test/java/com/swirlds/platform/test/event/linking/OrphanBufferingLinkerTest.java index d1448ecb9bb8..03af79be970c 100644 --- a/platform-sdk/swirlds-unit-tests/core/swirlds-platform-test/src/test/java/com/swirlds/platform/test/event/linking/OrphanBufferingLinkerTest.java +++ b/platform-sdk/swirlds-unit-tests/core/swirlds-platform-test/src/test/java/com/swirlds/platform/test/event/linking/OrphanBufferingLinkerTest.java @@ -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()); diff --git a/platform-sdk/swirlds-unit-tests/core/swirlds-platform-test/src/test/java/com/swirlds/platform/test/event/tipset/TipsetEventCreatorTests.java b/platform-sdk/swirlds-unit-tests/core/swirlds-platform-test/src/test/java/com/swirlds/platform/test/event/tipset/TipsetEventCreatorTests.java index a918da8255fc..982f81c8369e 100644 --- a/platform-sdk/swirlds-unit-tests/core/swirlds-platform-test/src/test/java/com/swirlds/platform/test/event/tipset/TipsetEventCreatorTests.java +++ b/platform-sdk/swirlds-unit-tests/core/swirlds-platform-test/src/test/java/com/swirlds/platform/test/event/tipset/TipsetEventCreatorTests.java @@ -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; @@ -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;