Skip to content

Commit d289ddb

Browse files
committed
refactor(spec): rename sendUncaughtExceptions -> autoDetectErrors
1 parent 32091c4 commit d289ddb

8 files changed

Lines changed: 186 additions & 58 deletions

File tree

bugsnag-spring/src/main/java/com/bugsnag/BugsnagAsyncExceptionHandler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public BugsnagAsyncExceptionHandler(final Bugsnag bugsnag) {
2424

2525
@Override
2626
public void handleUncaughtException(Throwable throwable, Method method, Object... obj) {
27-
if (bugsnag.getConfig().shouldSendUncaughtExceptions()) {
27+
if (bugsnag.getConfig().getAutoDetectErrors()) {
2828
HandledState handledState = HandledState.newInstance(
2929
SeverityReasonType.REASON_UNHANDLED_EXCEPTION_MIDDLEWARE,
3030
Collections.singletonMap("framework", "Spring"),

bugsnag-spring/src/main/java/com/bugsnag/BugsnagJakartaMvcExceptionHandler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public ModelAndView resolveException(HttpServletRequest request,
3333
Object handler,
3434
java.lang.Exception ex) {
3535

36-
if (bugsnag.getConfig().shouldSendUncaughtExceptions()) {
36+
if (bugsnag.getConfig().getAutoDetectErrors()) {
3737
HandledState handledState = HandledState.newInstance(
3838
SeverityReasonType.REASON_UNHANDLED_EXCEPTION_MIDDLEWARE,
3939
Collections.singletonMap("framework", "Spring"),

bugsnag-spring/src/main/java/com/bugsnag/BugsnagScheduledTaskExceptionHandler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public void handleError(Throwable throwable) {
2727
return;
2828
}
2929

30-
if (bugsnag.getConfig().shouldSendUncaughtExceptions()) {
30+
if (bugsnag.getConfig().getAutoDetectErrors()) {
3131
HandledState handledState = HandledState.newInstance(
3232
SeverityReasonType.REASON_UNHANDLED_EXCEPTION_MIDDLEWARE,
3333
Collections.singletonMap("framework", "Spring"),

bugsnag-spring/src/test/java/com/bugsnag/SpringMvcTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public void setUp() {
7171
delivery = mock(Delivery.class);
7272

7373
bugsnag.setDelivery(delivery);
74-
bugsnag.getConfig().setSendUncaughtExceptions(true);
74+
bugsnag.getConfig().setAutoDetectErrors(true);
7575
bugsnag.getConfig().setAutoCaptureSessions(true);
7676

7777
// Cannot reset the session count on the bugsnag bean for each test, so note
@@ -102,7 +102,7 @@ public void bugsnagNotifyWhenUncaughtControllerException() {
102102

103103
@Test
104104
public void noBugsnagNotifyWhenSendUncaughtExceptionsFalse() {
105-
bugsnag.getConfig().setSendUncaughtExceptions(false);
105+
bugsnag.getConfig().setAutoDetectErrors(false);
106106

107107
callRuntimeExceptionEndpoint();
108108

bugsnag/src/main/java/com/bugsnag/Bugsnag.java

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import org.slf4j.LoggerFactory;
1010

1111
import java.io.Closeable;
12-
import java.lang.Thread.UncaughtExceptionHandler;
1312
import java.net.Proxy;
1413
import java.util.Collection;
1514
import java.util.Collections;
@@ -61,12 +60,7 @@ public void rejectedExecution(Runnable runnable, ThreadPoolExecutor executor) {
6160
private final SessionTracker sessionTracker;
6261
private final FeatureFlagStore featureFlagStore;
6362

64-
private static final ThreadLocal<Metadata> THREAD_METADATA = new ThreadLocal<Metadata>() {
65-
@Override
66-
public Metadata initialValue() {
67-
return new Metadata();
68-
}
69-
};
63+
private static final ThreadLocal<Metadata> THREAD_METADATA = ThreadLocal.withInitial(Metadata::new);
7064

7165
//
7266
// Constructors
@@ -97,7 +91,7 @@ public Bugsnag(String apiKey, boolean sendUncaughtExceptions) {
9791
featureFlagStore = config.copyFeatureFlagStore();
9892

9993
// Automatically send unhandled exceptions to Bugsnag using this Bugsnag
100-
config.setSendUncaughtExceptions(sendUncaughtExceptions);
94+
config.setAutoDetectErrors(sendUncaughtExceptions);
10195
if (sendUncaughtExceptions) {
10296
ExceptionHandler.enable(this);
10397
}
@@ -681,13 +675,13 @@ SessionTracker getSessionTracker() {
681675
*
682676
* @return clients which catch uncaught exceptions
683677
*/
684-
public static Set<Bugsnag> uncaughtExceptionClients() {
685-
UncaughtExceptionHandler handler = Thread.getDefaultUncaughtExceptionHandler();
686-
if (handler instanceof ExceptionHandler) {
687-
ExceptionHandler bugsnagHandler = (ExceptionHandler) handler;
688-
return Collections.unmodifiableSet(bugsnagHandler.uncaughtExceptionClients());
678+
public static Iterable<Bugsnag> uncaughtExceptionClients() {
679+
ExceptionHandler handler = ExceptionHandler.getGlobalOrNull();
680+
if (handler == null) {
681+
return Collections.emptyList();
689682
}
690-
return Collections.emptySet();
683+
684+
return handler.uncaughtExceptionClients();
691685
}
692686

693687
void addOnSession(OnSession onSession) {

bugsnag/src/main/java/com/bugsnag/Configuration.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public class Configuration {
4747

4848
Set<OnErrorCallback> callbacks = new CopyOnWriteArraySet<>();
4949
private volatile boolean autoCaptureSessions = true;
50-
private volatile boolean sendUncaughtExceptions = true;
50+
private volatile boolean autoDetectErrors = true;
5151
private final FeatureFlagStore featureFlagStore = new FeatureFlagStore();
5252

5353
Configuration(String apiKey) {
@@ -111,12 +111,12 @@ public boolean shouldAutoCaptureSessions() {
111111
return autoCaptureSessions;
112112
}
113113

114-
public void setSendUncaughtExceptions(boolean sendUncaughtExceptions) {
115-
this.sendUncaughtExceptions = sendUncaughtExceptions;
114+
public void setAutoDetectErrors(boolean autoDetectErrors) {
115+
this.autoDetectErrors = autoDetectErrors;
116116
}
117117

118-
public boolean shouldSendUncaughtExceptions() {
119-
return sendUncaughtExceptions;
118+
public boolean getAutoDetectErrors() {
119+
return autoDetectErrors;
120120
}
121121

122122
/**
Lines changed: 159 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,56 @@
11
package com.bugsnag;
22

33
import java.lang.Thread.UncaughtExceptionHandler;
4-
import java.util.Set;
5-
import java.util.WeakHashMap;
4+
import java.lang.ref.WeakReference;
5+
import java.util.Arrays;
6+
import java.util.Collections;
7+
import java.util.Objects;
8+
import java.util.concurrent.atomic.AtomicBoolean;
9+
import java.util.concurrent.atomic.AtomicReference;
10+
import java.util.stream.Stream;
611

712

813
class ExceptionHandler implements UncaughtExceptionHandler {
14+
private static volatile ExceptionHandler singletonInstance = null;
15+
916
private final UncaughtExceptionHandler originalHandler;
10-
private final WeakHashMap<Bugsnag, Boolean> clientMap = new WeakHashMap<Bugsnag, Boolean>();
17+
private final AtomicReference<WeakReference<Bugsnag>[]> weakClients = new AtomicReference<>(null);
18+
private final AtomicBoolean installed = new AtomicBoolean(false);
19+
20+
/**
21+
* Returns all the Bugsnag instances associated with this ExceptionHandler.
22+
*
23+
* @return an immutable iterable of the Bugsnag instances associated with this ExceptionHandler (never null)
24+
*/
25+
Iterable<Bugsnag> uncaughtExceptionClients() {
26+
WeakReference<Bugsnag>[] clientRefs = weakClients.get();
27+
if (clientRefs == null || clientRefs.length == 0) {
28+
return Collections.emptySet();
29+
}
1130

12-
Set<Bugsnag> uncaughtExceptionClients() {
13-
return clientMap.keySet();
31+
return () -> Stream.of(clientRefs)
32+
.map(WeakReference::get)
33+
.filter(Objects::nonNull)
34+
.iterator();
1435
}
1536

1637
static void enable(Bugsnag bugsnag) {
17-
UncaughtExceptionHandler currentHandler = Thread.getDefaultUncaughtExceptionHandler();
38+
ExceptionHandler handler = getGlobalOrCreate();
39+
handler.add(bugsnag);
1840

19-
// Find or create the Bugsnag ExceptionHandler
20-
ExceptionHandler bugsnagHandler;
21-
if (currentHandler instanceof ExceptionHandler) {
22-
bugsnagHandler = (ExceptionHandler) currentHandler;
23-
} else {
24-
bugsnagHandler = new ExceptionHandler(currentHandler);
25-
Thread.setDefaultUncaughtExceptionHandler(bugsnagHandler);
41+
if (handler.installed.compareAndSet(false, true)) {
42+
Thread.setDefaultUncaughtExceptionHandler(handler);
2643
}
27-
28-
// Subscribe this bugsnag to uncaught exceptions
29-
bugsnagHandler.clientMap.put(bugsnag, true);
3044
}
3145

3246
static void disable(Bugsnag bugsnag) {
33-
// Find the Bugsnag ExceptionHandler
34-
UncaughtExceptionHandler currentHandler = Thread.getDefaultUncaughtExceptionHandler();
35-
if (currentHandler instanceof ExceptionHandler) {
36-
// Unsubscribe this bugsnag from uncaught exceptions
37-
ExceptionHandler bugsnagHandler = (ExceptionHandler) currentHandler;
38-
bugsnagHandler.clientMap.remove(bugsnag);
39-
40-
// Remove the Bugsnag ExceptionHandler if no clients are subscribed
41-
if (bugsnagHandler.clientMap.size() == 0) {
42-
Thread.setDefaultUncaughtExceptionHandler(bugsnagHandler.originalHandler);
43-
}
47+
ExceptionHandler handler = getGlobalOrNull();
48+
if (handler == null) {
49+
return; // No handler installed, so nothing to disable
50+
}
51+
52+
if (handler.remove(bugsnag)) {
53+
handler.cleanup();
4454
}
4555
}
4656

@@ -51,10 +61,13 @@ static void disable(Bugsnag bugsnag) {
5161
@Override
5262
public void uncaughtException(java.lang.Thread thread, Throwable throwable) {
5363
// Notify any subscribed clients of the uncaught exception
54-
for (Bugsnag bugsnag : clientMap.keySet()) {
55-
if (bugsnag.getConfig().shouldSendUncaughtExceptions()) {
64+
for (Bugsnag bugsnag : uncaughtExceptionClients()) {
65+
if (bugsnag.getConfig().getAutoDetectErrors()) {
5666
HandledState handledState = HandledState.newInstance(
57-
HandledState.SeverityReasonType.REASON_UNHANDLED_EXCEPTION, Severity.ERROR);
67+
HandledState.SeverityReasonType.REASON_UNHANDLED_EXCEPTION,
68+
Severity.ERROR
69+
);
70+
5871
bugsnag.notify(throwable, handledState, thread);
5972
}
6073
}
@@ -68,4 +81,120 @@ public void uncaughtException(java.lang.Thread thread, Throwable throwable) {
6881
throwable.printStackTrace(System.err);
6982
}
7083
}
84+
85+
@SuppressWarnings("unchecked")
86+
void add(Bugsnag bugsnag) {
87+
WeakReference<Bugsnag> newRef = new WeakReference<>(bugsnag);
88+
89+
while (true) {
90+
final WeakReference<Bugsnag>[] currentRefs = weakClients.get();
91+
92+
if (currentRefs != null) {
93+
// Create a new array with the new client added
94+
WeakReference<Bugsnag>[] newRefs = new WeakReference[currentRefs.length + 1];
95+
int index = 0;
96+
for (WeakReference<Bugsnag> ref : currentRefs) {
97+
// Copy existing non-garbage collected references
98+
// It's not as fast as System.arraycopy, but it avoids needing even more copies of the array
99+
if (ref.get() != null) {
100+
newRefs[index++] = ref;
101+
}
102+
}
103+
newRefs[index++] = newRef;
104+
105+
if (index < newRefs.length) {
106+
// If some references were garbage collected, create a smaller array
107+
newRefs = Arrays.copyOf(newRefs, index);
108+
}
109+
110+
// Attempt to update the reference atomically
111+
if (weakClients.compareAndSet(currentRefs, newRefs)) {
112+
return; // Successfully added the client
113+
}
114+
} else {
115+
WeakReference<Bugsnag>[] newRefs = new WeakReference[] {
116+
newRef
117+
};
118+
119+
// Attempt to update the reference atomically
120+
if (weakClients.compareAndSet(null, newRefs)) {
121+
return; // Successfully added the client
122+
}
123+
}
124+
}
125+
}
126+
127+
@SuppressWarnings("unchecked")
128+
boolean remove(Bugsnag bugsnag) {
129+
while (true) {
130+
final WeakReference<Bugsnag>[] currentRefs = weakClients.get();
131+
if (currentRefs == null || currentRefs.length == 0) {
132+
return false;
133+
}
134+
135+
// Create a new array with the specified client removed
136+
WeakReference<Bugsnag>[] newRefs = new WeakReference[currentRefs.length - 1];
137+
int index = 0;
138+
for (WeakReference<Bugsnag> ref : currentRefs) {
139+
Bugsnag client = ref.get();
140+
if (client != null && client != bugsnag) {
141+
if (index >= newRefs.length) {
142+
// if we reached this point, then 'bugsnag' wasn't in the list - this will always be
143+
// the last element in the list, and it wasn't the one we are trying to remove so we
144+
// can exit here
145+
return false;
146+
}
147+
newRefs[index++] = ref;
148+
}
149+
}
150+
151+
if (index < newRefs.length) {
152+
// If some references were garbage collected, create a smaller array
153+
newRefs = Arrays.copyOf(newRefs, index);
154+
}
155+
156+
if (weakClients.compareAndSet(currentRefs, newRefs)) {
157+
// Return true if the array changed (target removed or GC'd refs cleaned)
158+
return index < currentRefs.length;
159+
}
160+
}
161+
}
162+
163+
private void cleanup() {
164+
WeakReference<Bugsnag>[] refs = weakClients.get();
165+
if (refs != null && refs.length > 0) {
166+
// this instance still has clients
167+
return;
168+
}
169+
170+
if (isCurrentInstalledHandler() && installed.compareAndSet(true, false)) {
171+
Thread.setDefaultUncaughtExceptionHandler(originalHandler);
172+
}
173+
}
174+
175+
/**
176+
* Return true if this instance is currently the default uncaught exception handler. If this instance is installed
177+
* as a handler, but is not the outermost/current handler this will return false (meaning it is not safe to
178+
* uninstall this handler as it is possibly being delegated to by another crash handler).
179+
*
180+
* @return true if this instance is currently the default uncaught exception handler, false otherwise
181+
*/
182+
private boolean isCurrentInstalledHandler() {
183+
return Thread.getDefaultUncaughtExceptionHandler() == this;
184+
}
185+
186+
static ExceptionHandler getGlobalOrCreate() {
187+
if (singletonInstance == null) {
188+
synchronized (ExceptionHandler.class) {
189+
if (singletonInstance == null) {
190+
singletonInstance = new ExceptionHandler(Thread.getDefaultUncaughtExceptionHandler());
191+
}
192+
}
193+
}
194+
return singletonInstance;
195+
}
196+
197+
static ExceptionHandler getGlobalOrNull() {
198+
return singletonInstance;
199+
}
71200
}

bugsnag/src/test/java/com/bugsnag/BugsnagTest.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.net.Proxy;
2222

2323
import java.util.HashMap;
24+
import java.util.Iterator;
2425
import java.util.Map;
2526
import java.util.Set;
2627
import java.util.regex.Pattern;
@@ -69,8 +70,8 @@ public void testIgnoreClasses() {
6970

7071
// Ignore both (compile patterns for exact matches)
7172
bugsnag.setDiscardClasses(
72-
Pattern.compile(Pattern.quote(RuntimeException.class.getName())),
73-
Pattern.compile(Pattern.quote(TestException.class.getName()))
73+
Pattern.compile(Pattern.quote(RuntimeException.class.getName())),
74+
Pattern.compile(Pattern.quote(TestException.class.getName()))
7475
);
7576
assertFalse(bugsnag.notify(new RuntimeException()));
7677
assertFalse(bugsnag.notify(new TestException()));
@@ -519,8 +520,12 @@ public void testSerialization() {
519520

520521
@Test(expected = UnsupportedOperationException.class)
521522
public void testUncaughtHandlerModification() {
522-
Set<Bugsnag> bugsnags = Bugsnag.uncaughtExceptionClients();
523-
bugsnags.clear();
523+
Iterator<Bugsnag> iterator = Bugsnag.uncaughtExceptionClients().iterator();
524+
while (iterator.hasNext()) {
525+
//noinspection resource
526+
iterator.next();
527+
iterator.remove(); // not supported
528+
}
524529
}
525530

526531
// Test exception class

0 commit comments

Comments
 (0)