Skip to content

Commit

Permalink
[vscode] Improve runInTerminal and support linux
Browse files Browse the repository at this point in the history
Depends on D93874.

runInTerminal was using --wait-for, but it was some problems because it uses process polling looking for a single instance of the debuggee:

- it gets to know of the target late, which renders breakpoints in the main function almost impossible
- polling might fail if there are already other processes with the same name
- polling might also fail on some linux machine, as it's implemented with the ps command, and the ps command's args and output are not standard everywhere

As a better way to implement this so that it works well on Darwin and Linux, I'm using now the following process:

- lldb-vscode notices the runInTerminal, so it spawns lldb-vscode with a special flag --launch-target <target>. This flags tells lldb-vscode to wait to be attached and then it execs the target program. I'm using lldb-vscode itself to do this, because it makes finding the launcher program easier. Also no CMAKE INSTALL scripts are needed.
- Besides this, the debugger creates a temporary FIFO file where the launcher program will write its pid to. That way the debugger will be sure of which program to attach.
- Once attach happend, the debugger creates a second temporary file to notify the launcher program that it has been attached, so that it can then exec. I'm using this instead of using a signal or a similar mechanism because I don't want the launcher program to wait indefinitely to be attached in case the debugger crashed. That would pollute the process list with a lot of hanging processes. Instead, I'm setting a 20 seconds timeout (that's an overkill) and the launcher program seeks in intervals the second tepmorary file.

Some notes:
- I preferred not to use sockets because it requires a lot of code and I only need a pid. It would also require a lot of code when windows support is implemented.
- I didn't add Windows support, as I don't have a windows machine, but adding support for it should be easy, as the FIFO file can be implemented with a named pipe, which is standard on Windows and works pretty much the same way.

The existing test which didn't pass on Linux, now passes.

Differential Revision: https://reviews.llvm.org/D93951
  • Loading branch information
walter-erquinigo committed Jan 25, 2021
1 parent 71af5a1 commit 0f0462c
Show file tree
Hide file tree
Showing 13 changed files with 779 additions and 67 deletions.
Expand Up @@ -282,7 +282,8 @@ def launch(self, program=None, args=None, cwd=None, env=None,
trace=False, initCommands=None, preRunCommands=None,
stopCommands=None, exitCommands=None, terminateCommands=None,
sourcePath=None, debuggerRoot=None, launchCommands=None,
sourceMap=None, disconnectAutomatically=True, runInTerminal=False):
sourceMap=None, disconnectAutomatically=True, runInTerminal=False,
expectFailure=False):
'''Sending launch request to vscode
'''

Expand Down Expand Up @@ -317,14 +318,20 @@ def cleanup():
debuggerRoot=debuggerRoot,
launchCommands=launchCommands,
sourceMap=sourceMap,
runInTerminal=runInTerminal)
runInTerminal=runInTerminal,
expectFailure=expectFailure)

if expectFailure:
return response

if not (response and response['success']):
self.assertTrue(response['success'],
'launch failed (%s)' % (response['message']))
# We need to trigger a request_configurationDone after we've successfully
# attached a runInTerminal process to finish initialization.
if runInTerminal:
self.vscode.request_configurationDone()
return response


def build_and_launch(self, program, args=None, cwd=None, env=None,
Expand All @@ -340,7 +347,7 @@ def build_and_launch(self, program, args=None, cwd=None, env=None,
self.build_and_create_debug_adaptor()
self.assertTrue(os.path.exists(program), 'executable must exist')

self.launch(program, args, cwd, env, stopOnEntry, disableASLR,
return self.launch(program, args, cwd, env, stopOnEntry, disableASLR,
disableSTDIO, shellExpandArguments, trace,
initCommands, preRunCommands, stopCommands, exitCommands,
terminateCommands, sourcePath, debuggerRoot, runInTerminal=runInTerminal)
Expand Up @@ -612,7 +612,7 @@ def request_launch(self, program, args=None, cwd=None, env=None,
stopCommands=None, exitCommands=None,
terminateCommands=None ,sourcePath=None,
debuggerRoot=None, launchCommands=None, sourceMap=None,
runInTerminal=False):
runInTerminal=False, expectFailure=False):
args_dict = {
'program': program
}
Expand Down Expand Up @@ -660,9 +660,10 @@ def request_launch(self, program, args=None, cwd=None, env=None,
}
response = self.send_recv(command_dict)

# Wait for a 'process' and 'initialized' event in any order
self.wait_for_event(filter=['process', 'initialized'])
self.wait_for_event(filter=['process', 'initialized'])
if not expectFailure:
# Wait for a 'process' and 'initialized' event in any order
self.wait_for_event(filter=['process', 'initialized'])
self.wait_for_event(filter=['process', 'initialized'])
return response

def request_next(self, threadId):
Expand Down
Expand Up @@ -11,22 +11,47 @@
import lldbvscode_testcase
import time
import os
import subprocess
import shutil
import json
from threading import Thread


class TestVSCode_runInTerminal(lldbvscode_testcase.VSCodeTestCaseBase):

mydir = TestBase.compute_mydir(__file__)

@skipUnlessDarwin
def readPidMessage(self, fifo_file):
with open(fifo_file, "r") as file:
self.assertIn("pid", file.readline())

def sendDidAttachMessage(self, fifo_file):
with open(fifo_file, "w") as file:
file.write(json.dumps({"kind": "didAttach"}) + "\n")

def readErrorMessage(self, fifo_file):
with open(fifo_file, "r") as file:
return file.readline()

@skipIfWindows
@skipIfRemote
def test_runInTerminal(self):
'''
Tests the "runInTerminal" reverse request. It makes sure that the IDE can
launch the inferior with the correct environment variables and arguments.
'''
if "debug" in str(os.environ["LLDBVSCODE_EXEC"]).lower():
# We skip this test for debug builds because it takes too long parsing lldb's own
# debug info. Release builds are fine.
# Checking this environment variable seems to be a decent proxy for a quick
# detection
return
program = self.getBuildArtifact("a.out")
source = 'main.c'
self.build_and_launch(program, stopOnEntry=True, runInTerminal=True, args=["foobar"], env=["FOO=bar"])
self.build_and_launch(
program, stopOnEntry=True, runInTerminal=True, args=["foobar"],
env=["FOO=bar"])

breakpoint_line = line_number(source, '// breakpoint')

self.set_source_breakpoints(source, [breakpoint_line])
Expand All @@ -46,3 +71,87 @@ def test_runInTerminal(self):
# We verify we were able to set the environment
env = self.vscode.request_evaluate('foo')['body']['result']
self.assertIn('bar', env)

@skipIfWindows
@skipIfRemote
def test_runInTerminalInvalidTarget(self):
self.build_and_create_debug_adaptor()
response = self.launch(
"INVALIDPROGRAM", stopOnEntry=True, runInTerminal=True, args=["foobar"], env=["FOO=bar"], expectFailure=True)
self.assertFalse(response['success'])
self.assertIn("Could not create a target for a program 'INVALIDPROGRAM': unable to find executable",
response['message'])

@skipIfWindows
@skipIfRemote
def test_missingArgInRunInTerminalLauncher(self):
proc = subprocess.run([self.lldbVSCodeExec, "--launch-target", "INVALIDPROGRAM"],
capture_output=True, universal_newlines=True)
self.assertTrue(proc.returncode != 0)
self.assertIn('"--launch-target" requires "--comm-file" to be specified', proc.stderr)

@skipIfWindows
@skipIfRemote
def test_FakeAttachedRunInTerminalLauncherWithInvalidProgram(self):
comm_file = os.path.join(self.getBuildDir(), "comm-file")
os.mkfifo(comm_file)

proc = subprocess.Popen(
[self.lldbVSCodeExec, "--comm-file", comm_file, "--launch-target", "INVALIDPROGRAM"],
universal_newlines=True, stderr=subprocess.PIPE)

self.readPidMessage(comm_file)
self.sendDidAttachMessage(comm_file)
self.assertIn("No such file or directory", self.readErrorMessage(comm_file))

_, stderr = proc.communicate()
self.assertIn("No such file or directory", stderr)

@skipIfWindows
@skipIfRemote
def test_FakeAttachedRunInTerminalLauncherWithValidProgram(self):
comm_file = os.path.join(self.getBuildDir(), "comm-file")
os.mkfifo(comm_file)

proc = subprocess.Popen(
[self.lldbVSCodeExec, "--comm-file", comm_file, "--launch-target", "echo", "foo"],
universal_newlines=True, stdout=subprocess.PIPE)

self.readPidMessage(comm_file)
self.sendDidAttachMessage(comm_file)

stdout, _ = proc.communicate()
self.assertIn("foo", stdout)

@skipIfWindows
@skipIfRemote
def test_FakeAttachedRunInTerminalLauncherAndCheckEnvironment(self):
comm_file = os.path.join(self.getBuildDir(), "comm-file")
os.mkfifo(comm_file)

proc = subprocess.Popen(
[self.lldbVSCodeExec, "--comm-file", comm_file, "--launch-target", "env"],
universal_newlines=True, stdout=subprocess.PIPE,
env={**os.environ, "FOO": "BAR"})

self.readPidMessage(comm_file)
self.sendDidAttachMessage(comm_file)

stdout, _ = proc.communicate()
self.assertIn("FOO=BAR", stdout)

@skipIfWindows
@skipIfRemote
def test_NonAttachedRunInTerminalLauncher(self):
comm_file = os.path.join(self.getBuildDir(), "comm-file")
os.mkfifo(comm_file)

proc = subprocess.Popen(
[self.lldbVSCodeExec, "--comm-file", comm_file, "--launch-target", "echo", "foo"],
universal_newlines=True, stderr=subprocess.PIPE,
env={**os.environ, "LLDB_VSCODE_RIT_TIMEOUT_IN_MS": "1000"})

self.readPidMessage(comm_file)

_, stderr = proc.communicate()
self.assertIn("Timed out trying to get messages from the debug adaptor", stderr)
2 changes: 2 additions & 0 deletions lldb/tools/lldb-vscode/CMakeLists.txt
Expand Up @@ -27,10 +27,12 @@ add_lldb_tool(lldb-vscode
lldb-vscode.cpp
BreakpointBase.cpp
ExceptionBreakpoint.cpp
FifoFiles.cpp
FunctionBreakpoint.cpp
IOStream.cpp
JSONUtils.cpp
LLDBUtils.cpp
RunInTerminal.cpp
SourceBreakpoint.cpp
VSCode.cpp

Expand Down
91 changes: 91 additions & 0 deletions lldb/tools/lldb-vscode/FifoFiles.cpp
@@ -0,0 +1,91 @@
//===-- FifoFiles.cpp -------------------------------------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#if !defined(WIN32)
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>
#endif

#include <chrono>
#include <fstream>
#include <future>
#include <thread>

#include "llvm/Support/FileSystem.h"

#include "lldb/lldb-defines.h"

#include "FifoFiles.h"

using namespace llvm;

namespace lldb_vscode {

FifoFile::FifoFile(StringRef path) : m_path(path) {}

FifoFile::~FifoFile() {
#if !defined(WIN32)
unlink(m_path.c_str());
#endif
};

Expected<std::shared_ptr<FifoFile>> CreateFifoFile(StringRef path) {
#if defined(WIN32)
return createStringError(inconvertibleErrorCode(), "Unimplemented");
#else
if (int err = mkfifo(path.data(), 0600))
return createStringError(std::error_code(err, std::generic_category()),
"Couldn't create fifo file: %s", path.data());
return std::make_shared<FifoFile>(path);
#endif
}

FifoFileIO::FifoFileIO(StringRef fifo_file, StringRef other_endpoint_name)
: m_fifo_file(fifo_file), m_other_endpoint_name(other_endpoint_name) {}

Expected<json::Value> FifoFileIO::ReadJSON(std::chrono::milliseconds timeout) {
// We use a pointer for this future, because otherwise its normal destructor
// would wait for the getline to end, rendering the timeout useless.
Optional<std::string> line;
std::future<void> *future =
new std::future<void>(std::async(std::launch::async, [&]() {
std::ifstream reader(m_fifo_file, std::ifstream::in);
std::string buffer;
std::getline(reader, buffer);
if (!buffer.empty())
line = buffer;
}));
if (future->wait_for(timeout) == std::future_status::timeout ||
!line.hasValue())
return createStringError(inconvertibleErrorCode(),
"Timed out trying to get messages from the " +
m_other_endpoint_name);
delete future;
return json::parse(*line);
}

Error FifoFileIO::SendJSON(const json::Value &json,
std::chrono::milliseconds timeout) {
bool done = false;
std::future<void> *future =
new std::future<void>(std::async(std::launch::async, [&]() {
std::ofstream writer(m_fifo_file, std::ofstream::out);
writer << JSONToString(json) << std::endl;
done = true;
}));
if (future->wait_for(timeout) == std::future_status::timeout || !done) {
return createStringError(inconvertibleErrorCode(),
"Timed out trying to send messages to the " +
m_other_endpoint_name);
}
delete future;
return Error::success();
}

} // namespace lldb_vscode
84 changes: 84 additions & 0 deletions lldb/tools/lldb-vscode/FifoFiles.h
@@ -0,0 +1,84 @@
//===-- FifoFiles.h ---------------------------------------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef LLDB_TOOLS_LLDB_VSCODE_FIFOFILES_H
#define LLDB_TOOLS_LLDB_VSCODE_FIFOFILES_H

#include "llvm/Support/Error.h"

#include "JSONUtils.h"

namespace lldb_vscode {

/// Struct that controls the life of a fifo file in the filesystem.
///
/// The file is destroyed when the destructor is invoked.
struct FifoFile {
FifoFile(llvm::StringRef path);

~FifoFile();

std::string m_path;
};

/// Create a fifo file in the filesystem.
///
/// \param[in] path
/// The path for the fifo file.
///
/// \return
/// A \a std::shared_ptr<FifoFile> if the file could be created, or an
/// \a llvm::Error in case of failures.
llvm::Expected<std::shared_ptr<FifoFile>> CreateFifoFile(llvm::StringRef path);

class FifoFileIO {
public:
/// \param[in] fifo_file
/// The path to an input fifo file that exists in the file system.
///
/// \param[in] other_endpoint_name
/// A human readable name for the other endpoint that will communicate
/// using this file. This is used for error messages.
FifoFileIO(llvm::StringRef fifo_file, llvm::StringRef other_endpoint_name);

/// Read the next JSON object from the underlying input fifo file.
///
/// The JSON object is expected to be a single line delimited with \a
/// std::endl.
///
/// \return
/// An \a llvm::Error object indicating the success or failure of this
/// operation. Failures arise if the timeout is hit, the next line of text
/// from the fifo file is not a valid JSON object, or is it impossible to
/// read from the file.
llvm::Expected<llvm::json::Value> ReadJSON(std::chrono::milliseconds timeout);

/// Serialize a JSON object and write it to the underlying output fifo file.
///
/// \param[in] json
/// The JSON object to send. It will be printed as a single line delimited
/// with \a std::endl.
///
/// \param[in] timeout
/// A timeout for how long we should until for the data to be consumed.
///
/// \return
/// An \a llvm::Error object indicating whether the data was consumed by
/// a reader or not.
llvm::Error SendJSON(
const llvm::json::Value &json,
std::chrono::milliseconds timeout = std::chrono::milliseconds(20000));

private:
std::string m_fifo_file;
std::string m_other_endpoint_name;
};

} // namespace lldb_vscode

#endif // LLDB_TOOLS_LLDB_VSCODE_FIFOFILES_H

0 comments on commit 0f0462c

Please sign in to comment.