You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2020/10/02 04:09:14 UTC
[GitHub] [skywalking] muse-dev[bot] commented on a change in pull request #5618: [WIP]Fix thread safety violation
muse-dev[bot] commented on a change in pull request #5618:
URL: https://github.com/apache/skywalking/pull/5618#discussion_r498611103
##########
File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/boot/AgentPackagePath.java
##########
@@ -36,10 +36,12 @@
private static File AGENT_PACKAGE_PATH;
public static File getPath() throws AgentPackageNotFoundException {
- if (AGENT_PACKAGE_PATH == null) {
- AGENT_PACKAGE_PATH = findPath();
+ synchronized (AGENT_PACKAGE_PATH) {
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `AgentPackagePath.getPath()` reads without synchronization from `boot.AgentPackagePath.AGENT_PACKAGE_PATH`. Potentially races with write in method `AgentPackagePath.getPath()`.
Reporting because this access may occur on a background thread.
##########
File path: apm-commons/apm-datacarrier/src/main/java/org/apache/skywalking/apm/commons/datacarrier/consumer/ConsumerPoolFactory.java
##########
@@ -18,20 +18,19 @@
package org.apache.skywalking.apm.commons.datacarrier.consumer;
-import java.util.HashMap;
-import java.util.Map;
import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentHashMap;
/**
* Consumer Pool Factory provides global management for all Consumer Pool.
*/
public enum ConsumerPoolFactory {
INSTANCE;
- private final Map<String, ConsumerPool> pools;
+ private final ConcurrentHashMap<String, ConsumerPool> pools;
Review comment:
*ImmutableEnumChecker:* enums should be immutable: 'ConsumerPoolFactory' has field 'pools' of type 'java.util.concurrent.ConcurrentHashMap<java.lang.String,org.apache.skywalking.apm.commons.datacarrier.consumer.ConsumerPool>', the declaration of type 'java.util.concurrent.ConcurrentHashMap<java.lang.String,org.apache.skywalking.apm.commons.datacarrier.consumer.ConsumerPool>' is not annotated with @com.google.errorprone.annotations.Immutable
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org