Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(anr): Display offending thread stacktrace in case of deadlock #50925

Merged
merged 5 commits into from
Jun 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"type": "chrome",
"request": "launch",
"url": "http://dev.getsentry.net:8000",
"webRoot": "${workspaceRoot}/src/sentry/static/sentry/app"
romtsn marked this conversation as resolved.
Show resolved Hide resolved
"webRoot": "${workspaceRoot}/static"
},
{
"name": "sentry backend",
Expand Down
162 changes: 160 additions & 2 deletions static/app/components/events/interfaces/analyzeFrames.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import {analyzeFramesForRootCause} from 'sentry/components/events/interfaces/analyzeFrames';
import {EntryType, Event, EventOrGroupType, Frame} from 'sentry/types/event';
import {render, screen} from 'sentry-test/reactTestingLibrary';
import {textWithMarkupMatcher} from 'sentry-test/utils';

import {
analyzeFrameForRootCause,
analyzeFramesForRootCause,
} from 'sentry/components/events/interfaces/analyzeFrames';
import {EntryType, Event, EventOrGroupType, Frame, LockType} from 'sentry/types/event';

const makeEventWithFrames = (frames: Frame[]): Event => {
const event: Event = {
Expand Down Expand Up @@ -225,4 +231,156 @@ describe('analyzeAnrFrames', function () {
'/^android\\.app\\.SharedPreferencesImpl\\$EditorImpl\\$[0-9]/'
);
});

it('given lock address returns frame with matching lock address', function () {
const frame1 = {
filename: 'Instrumentation.java',
absPath: 'Instrumentation.java',
module: 'android.app.Instrumentation',
package: null,
platform: null,
instructionAddr: null,
symbolAddr: null,
function: 'call',
rawFunction: null,
symbol: null,
context: [],
lineNo: 1176,
colNo: null,
inApp: false,
trust: null,
errors: null,
vars: null,
lock: {
type: LockType.BLOCKED,
address: '0x08a8651c',
package_name: 'io.sentry.samples',
class_name: 'Monitor',
thread_id: 12,
},
};
const frame2 = {
filename: 'MainActivity.java',
absPath: 'MainActivity.java',
module: 'com.example.MainActivity',
package: null,
platform: null,
instructionAddr: null,
symbolAddr: null,
function: 'onCreate',
rawFunction: null,
symbol: null,
context: [],
lineNo: 128,
colNo: null,
inApp: false,
trust: null,
errors: null,
vars: null,
lock: {
type: LockType.BLOCKED,
address: '0x07d7437b',
package_name: 'java.lang',
class_name: 'Object',
thread_id: 7,
},
};
const rootCause1 = analyzeFrameForRootCause(frame1, undefined, '<address>');
expect(rootCause1).toBeNull();

const rootCause2 = analyzeFrameForRootCause(frame2, undefined, '0x07d7437b');
render(<div>{rootCause2?.resources}</div>);
expect(
screen.getByText(
textWithMarkupMatcher(
'The main thread is blocked/waiting, trying to acquire lock 0x07d7437b (java.lang.Object) held by the suspect frame of this thread.'
)
)
).toBeInTheDocument();
});

it('when thread id is not provided, does not append "held by"', function () {
const frame = {
filename: 'MainActivity.java',
absPath: 'MainActivity.java',
module: 'com.example.MainActivity',
package: null,
platform: null,
instructionAddr: null,
symbolAddr: null,
function: 'onCreate',
rawFunction: null,
symbol: null,
context: [],
lineNo: 128,
colNo: null,
inApp: false,
trust: null,
errors: null,
vars: null,
lock: {
type: LockType.BLOCKED,
address: '0x07d7437b',
package_name: 'java.lang',
class_name: 'Object',
},
};
const rootCause2 = analyzeFrameForRootCause(frame, undefined, '0x07d7437b');
render(<div>{rootCause2?.resources}</div>);
expect(
screen.getByText(
textWithMarkupMatcher(
'The main thread is blocked/waiting, trying to acquire lock 0x07d7437b (java.lang.Object) .'
)
)
).toBeInTheDocument();
});

it('given main thread is locked returns it as root cause', function () {
const frame = {
filename: 'MainActivity.java',
absPath: 'MainActivity.java',
module: 'com.example.MainActivity',
package: null,
platform: null,
instructionAddr: null,
symbolAddr: null,
function: 'onCreate',
rawFunction: null,
symbol: null,
context: [],
lineNo: 128,
colNo: null,
inApp: false,
trust: null,
errors: null,
vars: null,
lock: {
type: LockType.BLOCKED,
address: '0x08a1321b',
package_name: 'java.lang',
class_name: 'Object',
thread_id: 7,
},
};
const thread = {
id: 13920,
current: true,
crashed: true,
name: 'puma 002',
stacktrace: null,
rawStacktrace: null,
state: 'BLOCKED',
};
const rootCause = analyzeFrameForRootCause(frame, thread);

render(<div>{rootCause?.resources}</div>);
expect(
screen.getByText(
textWithMarkupMatcher(
'The main thread is blocked/waiting, trying to acquire lock 0x08a1321b (java.lang.Object) held by the suspect frame of this thread.'
)
)
).toBeInTheDocument();
});
});
69 changes: 57 additions & 12 deletions static/app/components/events/interfaces/analyzeFrames.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import styled from '@emotion/styled';
import isNil from 'lodash/isNil';

import {
Expand All @@ -7,7 +8,7 @@ import {
import {getCurrentThread} from 'sentry/components/events/interfaces/utils';
import ExternalLink from 'sentry/components/links/externalLink';
import {t, tct} from 'sentry/locale';
import {EntryException, EntryType, Event, Frame, Thread} from 'sentry/types';
import {EntryException, EntryType, Event, Frame, Lock, Thread} from 'sentry/types';
import {defined} from 'sentry/utils';

type SuspectFrame = {
Expand Down Expand Up @@ -180,24 +181,21 @@ function satisfiesFunctionCondition(frame: Frame, suspect: SuspectFrame) {

function satisfiesOffendingThreadCondition(
threadState: string | undefined | null,
suspect: SuspectFrame
offendingThreadStates?: ThreadStates[]
) {
if (
isNil(suspect.offendingThreadStates) ||
suspect.offendingThreadStates.length === 0
) {
if (isNil(offendingThreadStates) || offendingThreadStates.length === 0) {
return true;
}
const mappedState = getMappedThreadState(threadState);

if (isNil(mappedState)) {
return false;
}
return suspect.offendingThreadStates.includes(mappedState);
return offendingThreadStates.includes(mappedState);
}

export function analyzeFramesForRootCause(event: Event): {
culprit: string;
culprit: string | Lock;
resources: React.ReactNode;
} | null {
const exception = event.entries.find(entry => entry.type === EntryType.EXCEPTION) as
Expand All @@ -216,7 +214,7 @@ export function analyzeFramesForRootCause(event: Event): {
const currentThread = getCurrentThread(event);

// iterating the frames in reverse order, because the topmost frames most like the root cause
for (let index = exceptionFrames.length - 1; index > 0; index--) {
for (let index = exceptionFrames.length - 1; index >= 0; index--) {
const frame = exceptionFrames[index];
const rootCause = analyzeFrameForRootCause(frame, currentThread);
if (defined(rootCause)) {
Expand All @@ -227,18 +225,61 @@ export function analyzeFramesForRootCause(event: Event): {
return null;
}

function lockRootCauseCulprit(lock: Lock): {
culprit: string | Lock;
resources: React.ReactNode;
} {
const address = lock.address;
const obj = `${lock.package_name}.${lock.class_name}`;
const tid = lock.thread_id;
return {
culprit: lock,
resources: tct(
'The main thread is blocked/waiting, trying to acquire lock [address] ([obj]) [heldByThread]',
{
address: <Bold>{address}</Bold>,
obj: <Bold>{obj}</Bold>,
heldByThread: tid ? 'held by the suspect frame of this thread.' : '.',
}
),
};
}

export function analyzeFrameForRootCause(
frame: Frame,
currentThread: Thread | undefined
currentThread?: Thread,
lockAddress?: string
): {
culprit: string;
culprit: string | Lock;
resources: React.ReactNode;
} | null {
if (defined(lockAddress) && frame.lock?.address === lockAddress) {
// if we are provided with a lockAddress, we just have to analyze if the frame's lock
// address is equal to the one provided to mark the frame as suspect
return lockRootCauseCulprit(frame.lock);
}
if (
defined(frame.lock) &&
currentThread?.current &&
satisfiesOffendingThreadCondition(currentThread?.state, [
ThreadStates.WAITING,
ThreadStates.TIMED_WAITING,
ThreadStates.BLOCKED,
])
) {
// if the current (main) thread contains a lock and not in a RUNNABLE state, we return early
// with the lock being the culprit
return lockRootCauseCulprit(frame.lock);
}
// otherwise, we analyze for common patterns
for (const possibleCulprit of CULPRIT_FRAMES) {
if (
satisfiesModuleCondition(frame, possibleCulprit) &&
satisfiesFunctionCondition(frame, possibleCulprit) &&
satisfiesOffendingThreadCondition(currentThread?.state, possibleCulprit)
satisfiesOffendingThreadCondition(
currentThread?.state,
possibleCulprit.offendingThreadStates
)
) {
return {
culprit:
Expand All @@ -251,3 +292,7 @@ export function analyzeFrameForRootCause(
}
return null;
}

const Bold = styled('span')`
font-weight: bold;
`;
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type Props = {
meta?: Record<any, any>;
newestFirst?: boolean;
stackView?: StackTraceProps['stackView'];
threadId?: number;
} & Pick<ExceptionType, 'values'> &
Pick<
React.ComponentProps<typeof StackTrace>,
Expand Down Expand Up @@ -128,6 +129,7 @@ export function Content({
values,
type,
meta,
threadId,
}: Props) {
const {collapsedExceptions, toggleException, expandException} =
useCollapsedExceptions(values);
Expand Down Expand Up @@ -216,6 +218,7 @@ export function Content({
groupingCurrentLevel={groupingCurrentLevel}
meta={meta?.[excIdx]?.stacktrace}
debugFrames={hasSourcemapDebug ? debugFrames : undefined}
threadId={threadId}
/>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ type Props = {
groupingCurrentLevel?: Group['metadata']['current_level'];
meta?: Record<any, any>;
stackView?: StackView;
threadId?: number;
} & Pick<ExceptionType, 'values'>;

export function ExceptionContent({
Expand All @@ -29,6 +30,7 @@ export function ExceptionContent({
groupingCurrentLevel,
platform = 'other',
meta,
threadId,
}: Props) {
return (
<ErrorBoundary mini>
Expand All @@ -52,6 +54,7 @@ export function ExceptionContent({
hasHierarchicalGrouping={hasHierarchicalGrouping}
groupingCurrentLevel={groupingCurrentLevel}
meta={meta}
threadId={threadId}
/>
)}
</ErrorBoundary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type Props = {
meta?: Record<any, any>;
newestFirst?: boolean;
stackView?: StackView;
threadId?: number;
};

function StackTrace({
Expand All @@ -41,6 +42,7 @@ function StackTrace({
expandFirstFrame,
event,
meta,
threadId,
}: Props) {
if (!defined(stacktrace)) {
return null;
Expand Down Expand Up @@ -123,6 +125,7 @@ function StackTrace({
event={event}
meta={meta}
debugFrames={debugFrames}
threadId={threadId}
/>
);
}
Expand Down
Loading