[
https://issues.apache.org/jira/browse/SLING-9029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17026215#comment-17026215
]
Mohit Arora edited comment on SLING-9029 at 1/29/20 9:01 PM:
-------------------------------------------------------------
Hi [~marett],
Is there a valid case when {{receiveCallbacks}} can contain null? Checking
nullability of {{packageId}} would work now but if
[DefaultDistributionEventFactory#generatePackageEvent()|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/event/impl/DefaultDistributionEventFactory.java#L52]
also starts sending {{packageId}} as part of the event in future, thus leading
to the {{packageId}} being not null, we would start getting NPE again. I can
check for both packageId being not null and the existence of packageId in
{{receiveCallbacks}}. I will modify the PR accordingly along with a test case.
EDIT - I just realized that receiveCallbacks.remove() will just return null if
the key is not present and that null is already checked later in the function.
In that case I agree that just checking packageId being not null would suffice.
was (Author: mohiaror):
Hi [~marett],
Is there a valid case when {{receiveCallbacks}} can contain null? Checking
nullability of {{packageId}} would work now but if
[DefaultDistributionEventFactory#generatePackageEvent()|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/event/impl/DefaultDistributionEventFactory.java#L52]
also starts sending {{packageId}} as part of the event in future, thus leading
to the {{packageId}} being not null, we would start getting NPE again. I can
check for both packageId being not null and the existence of packageId in
{{receiveCallbacks}}. I will modify the PR accordingly along with a test case.
> Warning logged by PackageQueuedNotifier if a DistributionPackage does not
> exist in the callback map
> ---------------------------------------------------------------------------------------------------
>
> Key: SLING-9029
> URL: https://issues.apache.org/jira/browse/SLING-9029
> Project: Sling
> Issue Type: Bug
> Components: Content Distribution
> Reporter: Mohit Arora
> Assignee: Timothee Maret
> Priority: Major
> Fix For: Content Distribution Journal Core 0.1.8
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> [PackageQueuedNotifier.java|https://github.com/apache/sling-org-apache-sling-distribution-journal/blob/master/src/main/java/org/apache/sling/distribution/journal/impl/publisher/PackageQueuedNotifier.java#L66]
> handles all the {{DistributionEventTopics.AGENT_PACKAGE_QUEUED}} events and
> try to remove the distribution package associated with the event from a
> callback map which does not contain the package as the map is populated only
> in case of [journal based
> distribution|https://github.com/apache/sling-org-apache-sling-distribution-journal/blob/master/src/main/java/org/apache/sling/distribution/journal/impl/publisher/DistributionPublisher.java#L314]
> but the event is thrown in general from
> [QueueingDistributionPackageProcessor.java|https://github.com/apache/sling-org-apache-sling-distribution-core/blob/master/src/main/java/org/apache/sling/distribution/agent/impl/QueueingDistributionPackageProcessor.java#L128]
> which can be called for a normal end to end distribution without using the
> kafka pipeline and the journal flow.
> In such a case, a warning like below is logged -
> {code}07.01.2020 10:10:35.831 *WARN* [FelixLogListener]
> org.apache.felix.eventadmin Service
> [org.apache.sling.distribution.journal.impl.publisher.PackageQueuedNotifier,2499,
> [org.apache.sling.distribution.journal.impl.publisher.PackageQueuedNotifier,
> org.osgi.service.event.EventHandler]] EventAdmin: Exception during event
> dispatch [org.osgi.service.event.Event
> [topic=org/apache/sling/distribution/agent/package/queued]
> {distribution.type=ADD, distribution.component.name=bpuitestingtenant0,
> distribution.component.kind=AGENT,
> distribution.paths=[Ljava.lang.String;@5dd0dbd0} |
> [org.apache.sling.distribution.journal.impl.publisher.PackageQueuedNotifier,
> org.osgi.service.event.EventHandler] |
> Bundle(org.apache.sling.distribution.journal [527])]
> (java.lang.NullPointerException)
> java.lang.NullPointerException: null
> at
> java.base/java.util.concurrent.ConcurrentHashMap.replaceNode(ConcurrentHashMap.java:1111)
> at
> java.base/java.util.concurrent.ConcurrentHashMap.remove(ConcurrentHashMap.java:1102)
> at
> org.apache.sling.distribution.journal.impl.publisher.PackageQueuedNotifier.handleEvent(PackageQueuedNotifier.java:66)
> [org.apache.sling.distribution.journal:0.1.6]
> at
> org.apache.felix.eventadmin.impl.handler.EventHandlerProxy.sendEvent(EventHandlerProxy.java:415)
> [org.apache.felix.eventadmin:1.5.0]
> at
> org.apache.felix.eventadmin.impl.tasks.HandlerTask.runWithoutBlacklistTiming(HandlerTask.java:82)
> [org.apache.felix.eventadmin:1.5.0]
> at
> org.apache.felix.eventadmin.impl.tasks.SyncDeliverTasks.execute(SyncDeliverTasks.java:104)
> [org.apache.felix.eventadmin:1.5.0]
> at
> org.apache.felix.eventadmin.impl.tasks.AsyncDeliverTasks$TaskExecuter.run(AsyncDeliverTasks.java:166)
> [org.apache.felix.eventadmin:1.5.0]
> at
> java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
> at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
> at
> java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
> at
> java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
> at java.base/java.lang.Thread.run(Thread.java:834){code}
> Ideally, there should be different event(s) generated for different types of
> distribution.
> cc - [~ashishc], [~marett]
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
