Skip to content

Commit 0b5fbca

Browse files
littleajlittleaj
authored
Merge pull request #996 from microsoft/static_analysis_fixes
Static analysis fixes
2 parents a369830 + 9e873c0 commit 0b5fbca

File tree

12 files changed

+71
-79
lines changed

12 files changed

+71
-79
lines changed

‎core/src/main/java/com/microsoft/applicationinsights/channel/concrete/TelemetryChannelBase.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ public TelemetryChannelBase(Map<String, String> namesAndValues) {
154154
LimitsEnforcer sendIntervalInSecondsEnforcer = createDefaultSendIntervalInSecondsEnforcer(null);
155155

156156
boolean throttling = true;
157+
String maxTransmissionStorageCapacity = null;
157158
if (namesAndValues != null) {
158159
throttling = Boolean.valueOf(namesAndValues.get(THROTTLING_ENABLED_NAME));
159160
developerMode = Boolean.valueOf(namesAndValues.get(DEVELOPER_MODE_NAME));
@@ -172,14 +173,11 @@ public TelemetryChannelBase(Map<String, String> namesAndValues) {
172173
}
173174
endpointAddress = namesAndValues.get(ENDPOINT_ADDRESS_NAME);
174175

175-
maxTelemetryBufferCapacityEnforcer
176-
.normalizeStringValue(namesAndValues.get(MAX_TELEMETRY_BUFFER_CAPACITY_NAME));
177-
sendIntervalInSecondsEnforcer
178-
.normalizeStringValue(namesAndValues.get(FLUSH_BUFFER_TIMEOUT_IN_SECONDS_NAME));
176+
maxTelemetryBufferCapacityEnforcer.normalizeStringValue(namesAndValues.get(MAX_TELEMETRY_BUFFER_CAPACITY_NAME));
177+
sendIntervalInSecondsEnforcer.normalizeStringValue(namesAndValues.get(FLUSH_BUFFER_TIMEOUT_IN_SECONDS_NAME));
178+
maxTransmissionStorageCapacity = namesAndValues.get(MAX_TRANSMISSION_STORAGE_CAPACITY_NAME);
179179
}
180180

181-
String maxTransmissionStorageCapacity =
182-
namesAndValues.get(MAX_TRANSMISSION_STORAGE_CAPACITY_NAME);
183181
initialize(
184182
endpointAddress,
185183
maxTransmissionStorageCapacity,

‎core/src/main/java/com/microsoft/applicationinsights/channel/concrete/localforwarder/LocalForwarderTelemetryChannel.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ protected boolean doSend(com.microsoft.applicationinsights.telemetry.Telemetry t
5454
BaseTelemetry base = (BaseTelemetry) telemetry;
5555
if (base == null) {
5656
InternalLogger.INSTANCE.warn("Received null telemetry item. Skipping...");
57+
return false;
5758
}
5859

5960
try {

‎core/src/main/java/com/microsoft/applicationinsights/extensibility/initializer/docker/internal/DockerContextPoller.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,9 @@ public void run() {
5757
if (!fileExists) {
5858
try {
5959
Thread.sleep(THREAD_POLLING_INTERVAL_MS);
60-
61-
continue;
6260
} catch (InterruptedException e) {
63-
InternalLogger.INSTANCE.error("Error while executing docker context poller");
64-
InternalLogger.INSTANCE.trace("Stack trace generated is %s", ExceptionUtils.getStackTrace(e));
61+
Thread.currentThread().interrupt();
62+
break;
6563
}
6664
}
6765
}

‎core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/ActiveTransmissionLoader.java‎

Lines changed: 33 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,7 @@
2121

2222
package com.microsoft.applicationinsights.internal.channel.common;
2323

24-
import java.util.concurrent.BrokenBarrierException;
25-
import java.util.concurrent.CyclicBarrier;
24+
import java.util.concurrent.CountDownLatch;
2625
import java.util.concurrent.TimeUnit;
2726
import java.util.concurrent.atomic.AtomicBoolean;
2827

@@ -39,12 +38,12 @@
3938
* Created by gupele on 12/22/2014.
4039
*/
4140
public final class ActiveTransmissionLoader implements TransmissionsLoader {
42-
public final static int MAX_THREADS_ALLOWED = 10;
41+
public static final int MAX_THREADS_ALLOWED = 10;
4342

44-
private final static int DEFAULT_NUMBER_OF_THREADS = 1;
43+
private static final int DEFAULT_NUMBER_OF_THREADS = 1;
4544

46-
private final static long DEFAULT_SLEEP_INTERVAL_WHEN_NO_TRANSMISSIONS_FOUND_IN_MILLS = 2000;
47-
private final static long DEFAULT_SLEEP_INTERVAL_AFTER_DISPATCHING_IN_MILLS = 100;
45+
private static final long DEFAULT_SLEEP_INTERVAL_WHEN_NO_TRANSMISSIONS_FOUND_IN_MILLS = 2000;
46+
private static final long DEFAULT_SLEEP_INTERVAL_AFTER_DISPATCHING_IN_MILLS = 100;
4847

4948
// The helper class that encapsulates the file system access
5049
private final TransmissionFileSystemOutput fileSystem;
@@ -55,7 +54,7 @@ public final class ActiveTransmissionLoader implements TransmissionsLoader {
5554
// The dispatcher is needed to process the fetched Transmissions
5655
private final TransmissionDispatcher dispatcher;
5756

58-
private CyclicBarrier barrier;
57+
private final CountDownLatch latch;
5958

6059
private final TransmissionPolicyStateFetcher transmissionPolicyFetcher;
6160

@@ -86,18 +85,13 @@ public ActiveTransmissionLoader(final TransmissionFileSystemOutput fileSystem,
8685
this.fileSystem = fileSystem;
8786
this.dispatcher = dispatcher;
8887
threads = new Thread[numberOfThreads];
88+
latch = new CountDownLatch(numberOfThreads);
8989
final String threadNameFmt = String.format("%s-worker-%%d", ActiveTransmissionLoader.class.getSimpleName());
9090
for (int i = 0; i < numberOfThreads; ++i) {
9191
threads[i] = new Thread(new Runnable() {
9292
@Override
9393
public void run() {
94-
try {
95-
barrier.await();
96-
} catch (InterruptedException e) {
97-
InternalLogger.INSTANCE.error("Interrupted during barrier wait, exception: %s", e.toString());
98-
} catch (BrokenBarrierException e) {
99-
InternalLogger.INSTANCE.error("Failed during barrier wait, exception: %s", e.toString());
100-
}
94+
latch.countDown();
10195

10296
// Avoid un-expected exit of threads
10397
while (!done.get()) {
@@ -123,12 +117,14 @@ public void run() {
123117
Thread.sleep(DEFAULT_SLEEP_INTERVAL_AFTER_DISPATCHING_IN_MILLS);
124118
break;
125119
}
126-
} catch (Exception e) {
120+
} catch (InterruptedException e) {
121+
Thread.currentThread().interrupt();
122+
break;
127123
} catch (ThreadDeath td) {
128124
throw td;
129125
} catch (Throwable t) {
126+
// chomp
130127
}
131-
// TODO: check whether we need to pause after exception
132128
}
133129
}
134130
}, String.format(threadNameFmt, i));
@@ -137,23 +133,20 @@ public void run() {
137133

138134
@Override
139135
public synchronized boolean load(boolean waitForThreadsToStart) {
140-
if (barrier == null) {
141-
int numberOfThreads = threads.length;
142-
if (waitForThreadsToStart) {
143-
++numberOfThreads;
144-
}
145-
barrier = new CyclicBarrier(numberOfThreads);
146-
}
147136
for (Thread thread : threads) {
148137
thread.start();
149138
}
139+
140+
if (!waitForThreadsToStart) {
141+
return true;
142+
}
143+
150144
try {
151-
barrier.await();
145+
latch.await();
152146
return true;
153147
} catch (InterruptedException e) {
154-
InternalLogger.INSTANCE.error("Interrupted during barrier wait, exception: %s", e.toString());
155-
} catch (BrokenBarrierException e) {
156-
InternalLogger.INSTANCE.error("Failed during barrier wait, exception: %s", e.toString());
148+
InternalLogger.INSTANCE.error("Interrupted waiting for threads to start: %s", e.toString());
149+
Thread.currentThread().interrupt();
157150
}
158151

159152
return false;
@@ -162,16 +155,28 @@ public synchronized boolean load(boolean waitForThreadsToStart) {
162155
@Override
163156
public void stop(long timeout, TimeUnit timeUnit) {
164157
done.set(true);
158+
interruptAllThreads();
159+
joinAllThreads();
160+
}
161+
162+
private void joinAllThreads() {
165163
for (Thread thread : threads) {
166164
try {
167-
thread.interrupt();
168165
thread.join();
169166
} catch (InterruptedException e) {
170167
InternalLogger.INSTANCE.error("Interrupted during join of active transmission loader, exception: %s", e.toString());
168+
Thread.currentThread().interrupt();
169+
break;
171170
}
172171
}
173172
}
174173

174+
private void interruptAllThreads() {
175+
for (Thread thread : threads) {
176+
thread.interrupt();
177+
}
178+
}
179+
175180
private void fetchNext(boolean shouldDispatch) throws InterruptedException {
176181
Transmission transmission = fileSystem.fetchOldestFile();
177182
if (transmission == null) {
@@ -181,7 +186,6 @@ private void fetchNext(boolean shouldDispatch) throws InterruptedException {
181186
dispatcher.dispatch(transmission);
182187
}
183188

184-
// TODO: check if we need this as configuration value
185189
Thread.sleep(DEFAULT_SLEEP_INTERVAL_AFTER_DISPATCHING_IN_MILLS);
186190
}
187191
}

‎core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/PartialSuccessHandler.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ boolean validateTransmissionAndSend(TransmissionHandlerArgs args) {
112112
List<String> generateOriginalItems(TransmissionHandlerArgs args) {
113113
List<String> originalItems = new ArrayList<String>();
114114

115-
if (args.getTransmission().getWebContentEncodingType() == "gzip") {
115+
if ("gzip".equalsIgnoreCase(args.getTransmission().getWebContentEncodingType())) {
116116

117117
GZIPInputStream gis = null;
118118
BufferedReader bufferedReader = null;

‎core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/SenderThreadLocalBackOffData.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,8 @@ public boolean backOff() {
114114
backOffCondition.await(millisecondsToWait, TimeUnit.MILLISECONDS);
115115
return instanceIsActive;
116116
} catch (InterruptedException e) {
117-
return false;
117+
Thread.currentThread().interrupt();
118+
return false;
118119
}
119120
} finally {
120121
lock.unlock();

‎core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/TransmissionFileSystemOutput.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ public Transmission fetchOldestFile() {
193193
try {
194194
Thread.sleep(DELETE_TIMEOUT_ON_FAILURE_IN_MILLS);
195195
} catch (InterruptedException e) {
196+
Thread.currentThread().interrupt();
196197
break;
197198
}
198199
}

‎core/src/main/java/com/microsoft/applicationinsights/internal/config/ConfigurationFileLocator.java‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,8 @@ public InputStream getConfigurationFile() {
102102
}
103103

104104
private static void logException(Throwable t, String message) {
105-
if (t.getCause() != null) {
106-
InternalLogger.INSTANCE.warn("Failed to find configuration file, exception while fetching from %s: " +
107-
"Exception : '%s'",
108-
message, ExceptionUtils.getStackTrace(t));
109-
} else {
110-
InternalLogger.INSTANCE.warn("Failed to find configuration file, exception while fetching from %s: " +
111-
"Exception : '%s'",
112-
message, ExceptionUtils.getStackTrace(t));
105+
if (InternalLogger.INSTANCE.isWarnEnabled()) {
106+
InternalLogger.INSTANCE.warn("Failed to find configuration file, exception while fetching from %s: %s", message, ExceptionUtils.getStackTrace(t));
113107
}
114108
}
115109

‎core/src/main/java/com/microsoft/applicationinsights/internal/jmx/JmxDataFetcher.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,12 @@
2929
import java.util.Map;
3030
import java.util.HashMap;
3131

32+
import javax.management.AttributeNotFoundException;
33+
import javax.management.InstanceNotFoundException;
34+
import javax.management.MBeanException;
3235
import javax.management.MBeanServer;
3336
import javax.management.ObjectName;
37+
import javax.management.ReflectionException;
3438
import javax.management.openmbean.CompositeDataSupport;
3539
import java.lang.management.ManagementFactory;
3640

@@ -80,10 +84,9 @@ public static Map<String, Collection<Object>> fetch(String objectName, Collectio
8084
return result;
8185
}
8286

83-
private static Collection<Object> fetch(MBeanServer server, Set<ObjectName> objects, String attributeName, String attributeType) throws Exception {
87+
private static Collection<Object> fetch(MBeanServer server, Set<ObjectName> objects, String attributeName, String attributeType) throws AttributeNotFoundException, MBeanException, ReflectionException, InstanceNotFoundException {
8488
ArrayList<Object> result = new ArrayList<Object>();
8589

86-
String attr = attributeName;
8790
String[] inners = null;
8891

8992
AttributeType innerAttributeType = AttributeType.REGULAR;
@@ -101,7 +104,7 @@ private static Collection<Object> fetch(MBeanServer server, Set<ObjectName> obje
101104

102105
Object obj;
103106

104-
if (innerAttributeType != AttributeType.REGULAR) {
107+
if (inners != null) { // implies innerAttributeType != AttributeType.REGULAR
105108
obj = server.getAttribute(object, inners[0]);
106109

107110
javax.management.openmbean.CompositeDataSupport compositeData = null;
@@ -114,7 +117,7 @@ private static Collection<Object> fetch(MBeanServer server, Set<ObjectName> obje
114117
obj = compositeData.get(inners[1]);
115118
}
116119
} else {
117-
obj = server.getAttribute(object, attr);
120+
obj = server.getAttribute(object, attributeName);
118121
}
119122
if (obj != null) {
120123
result.add(obj);

‎core/src/main/java/com/microsoft/applicationinsights/internal/quickpulse/DefaultQuickPulseCoordinator.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,14 @@ public void run() {
5858
} else {
5959
sleepInMS = sendData();
6060
}
61-
try {
62-
Thread.sleep(sleepInMS);
63-
} catch (InterruptedException e) {
64-
}
61+
Thread.sleep(sleepInMS);
6562
}
63+
} catch (InterruptedException e) {
64+
Thread.currentThread().interrupt();
6665
} catch (ThreadDeath td) {
6766
throw td;
6867
} catch (Throwable t) {
68+
// chomp
6969
}
7070
}
7171

@@ -114,6 +114,6 @@ private long ping() {
114114
}
115115

116116
public void stop() {
117-
stopped = true;
117+
stopped = true;
118118
}
119119
}

0 commit comments

Comments
 (0)