Skip to content

Commit

Permalink
multithread test race condition fix
Browse files Browse the repository at this point in the history
  • Loading branch information
Luke-zhang-mchp committed Nov 13, 2024
1 parent 5ed7bbb commit c0c8672
Show file tree
Hide file tree
Showing 3 changed files with 204 additions and 2 deletions.
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -119,5 +119,6 @@
"src/native/*.ts",
"install.js",
"binding.gyp"
]
],
"packageManager": "[email protected]+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e"
}
6 changes: 5 additions & 1 deletion src/integration-tests/logpoints.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { CdtDebugClient } from './debugClient';
import {
fillDefaults,
gdbAsync,
isRemoteTest,
standardBeforeEach,
testProgramsDir,
} from './utils';
Expand All @@ -32,7 +33,10 @@ describe('logpoints', async () => {
});

afterEach(async () => {
if (!gdbAsync) {
// Fix race condition discussed in
// https://github.com/eclipse-cdt-cloud/cdt-gdb-adapter/pull/339#pullrequestreview-2427636654:
// logpoints
if (!gdbAsync && isRemoteTest) {
const waitForContinued = dc.waitForEvent('continued');
const threads = await dc.threadsRequest();
const pr = dc.continueRequest({
Expand Down
197 changes: 197 additions & 0 deletions src/integration-tests/multithread.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,22 @@ import {
} from './utils';
import { assert, expect } from 'chai';
import * as path from 'path';
import { fail } from 'assert';
import * as os from 'os';

describe('multithread', async function () {
let dc: CdtDebugClient;
const program = path.join(testProgramsDir, 'MultiThread');
const source = path.join(testProgramsDir, 'MultiThread.cc');

const threadNames = {
monday: 0,
tuesday: 1,
wednesday: 2,
thursday: 3,
friday: 4,
};

const lineTags = {
LINE_MAIN_ALL_THREADS_STARTED: 0,
LINE_THREAD_IN_HELLO: 0,
Expand All @@ -43,6 +52,139 @@ describe('multithread', async function () {
await dc.stop();
});

it('sees all threads', async function () {
if (!gdbNonStop && os.platform() === 'win32' && isRemoteTest) {
// The way thread names are set in remote tests on windows is unsupported
this.skip();
}

await dc.hitBreakpoint(
fillDefaults(this.test, {
program: program,
}),
{
path: source,
line: lineTags['LINE_MAIN_ALL_THREADS_STARTED'],
}
);

const threads = await dc.threadsRequest();
const nameToId = new Map(
threads.body.threads.map((thread) => [thread.name, thread.id])
);
// Make sure all 5 threads are there
expect(nameToId).to.include.keys(Object.keys(threadNames));
// and make sure that there is at least 6 threads.
// We don't care about the name of the "main" thread
expect(threads.body.threads).length.greaterThanOrEqual(6);

// check that each thread can be communicated with individually
for (const [name, idInProgram] of Object.entries(threadNames)) {
// There are multiple ids/indexes.
// idInProgram cooresponds to the variable thread_id in the C++ source
// threadId is the id of the thread in DAP
const threadId = nameToId.get(name);
if (threadId === undefined) {
// unreachable because of expect above
fail('unreachable');
}

if (gdbNonStop) {
const waitForStopped = dc.waitForEvent('stopped');
const pr = dc.pauseRequest({ threadId });
await Promise.all([pr, waitForStopped]);
}

const stack = await dc.stackTraceRequest({ threadId });
let printHelloFrameId: number | undefined = undefined;
let callerFrameId: number | undefined = undefined;
for (const frame of stack.body.stackFrames) {
if (frame.name === 'PrintHello') {
printHelloFrameId = frame.id;
} else if (printHelloFrameId !== undefined) {
callerFrameId = frame.id;
break;
}
}
if (printHelloFrameId === undefined) {
fail("Failed to find frame with name 'PrintHello'");
}
if (callerFrameId === undefined) {
fail("Failed to find frame that called 'PrintHello'");
}

{
const scopes = await dc.scopesRequest({
frameId: callerFrameId,
});
const vr = scopes.body.scopes[0].variablesReference;
const vars = await dc.variablesRequest({
variablesReference: vr,
});
const varnameToValue = new Map(
vars.body.variables.map((variable) => [
variable.name,
variable.value,
])
);
// Make sure we aren't getting the HelloWorld frame's variables.
// The calling method (in glibc or similar) may end up with a local
// variable called thread_id, if so, update this heuristic
expect(varnameToValue.get('thread_id')).to.be.undefined;
}
{
const scopes = await dc.scopesRequest({
frameId: printHelloFrameId,
});
const vr = scopes.body.scopes[0].variablesReference;
const vars = await dc.variablesRequest({
variablesReference: vr,
});
const varnameToValue = new Map(
vars.body.variables.map((variable) => [
variable.name,
variable.value,
])
);
expect(varnameToValue.get('thread_id')).to.equal(
idInProgram.toString()
);
// The "name" variable is a pointer, so is displayed as an address + the
// extracted nul terminated string
expect(varnameToValue.get('name')).to.contain(name);
}
{
// Make sure we can get variables for frame 0,
// the contents of those variables don't actually matter
// as the thread will probably be stopped in a library
// somewhere waiting for a semaphore
// This is a test for #235
const scopes = await dc.scopesRequest({
frameId: stack.body.stackFrames[0].id,
});
const vr = scopes.body.scopes[0].variablesReference;

const vars = await dc.variablesRequest({
variablesReference: vr,
});
const varnameToValue = new Map(
vars.body.variables.map((variable) => [
variable.name,
variable.value,
])
);
// Make sure we aren't getting the HelloWorld frame's variables.
// The calling method (in glibc or similar) may end up with a local
// variable called thread_id, if so, update this heuristic
// We could be stopped PrintHello, so we don't perform the check
// if that is the case
if (stack.body.stackFrames[0].id !== printHelloFrameId) {
expect(varnameToValue.get('thread_id')).to.be.undefined;
}
}
}
});

it('async resume for gdb-non-stop off', async function () {
if (gdbNonStop) {
// This test is covering only gdb-non-stop off mode
Expand Down Expand Up @@ -91,5 +233,60 @@ describe('multithread', async function () {
threadId: threads.body.threads[0].id,
allThreadsContinued: true,
});

if (!gdbAsync && isRemoteTest) {
const threads = await dc.threadsRequest();
await Promise.all(
threads.body.threads.map((thread) =>
dc.pauseRequest({
threadId: thread.id,
})
)
);
}
});

it('async resume for gdb-non-stop on', async function () {
if (!gdbNonStop) {
// This test is covering only gdb-non-stop on
this.skip();
}

await dc.hitBreakpoint(
fillDefaults(this.test, {
program: program,
}),
{
path: source,
line: lineTags['LINE_MAIN_ALL_THREADS_STARTED'],
}
);

const threads = await dc.threadsRequest();

// make sure that there is at least 6 threads.
expect(threads.body.threads).length.greaterThanOrEqual(6);

// stop the running threads
const runningThreads = threads.body.threads.filter(
(t) => (t as unknown as { running?: boolean }).running
);
for (const thread of runningThreads) {
await dc.pauseRequest({ threadId: thread.id });
await dc.waitForEvent('stopped');
}

for (const thread of threads.body.threads) {
// Send an async continue request and wait for the continue event.
dc.send('cdt-gdb-tests/executeCommand', {
command: `-exec-continue --thread ${thread.id}`,
});
const event = await dc.waitForEvent('continued');

assert.deepEqual<any>(event.body, {
threadId: thread.id,
allThreadsContinued: false,
});
}
});
});

0 comments on commit c0c8672

Please sign in to comment.