Skip to content

Commit 7e6c913

Browse files
authored
[lldb] Fix EditlineTest closing files multiple times. (#169100)
This updates the EditlineTest to use `lldb::FileSP` to ensure the associated FDs are only closed a single time. Currently, there is some confusion between the `FilePointer`, `PseudoTerminal` and `LockableStreamFile` about when the files are closed resulting in a crash in some due to a `fflush` on a closed file.
1 parent 9fa7627 commit 7e6c913

File tree

1 file changed

+28
-58
lines changed

1 file changed

+28
-58
lines changed

lldb/unittests/Editline/EditlineTest.cpp

Lines changed: 28 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
#include "lldb/Host/Config.h"
1010
#include "lldb/Host/File.h"
1111
#include "lldb/Host/HostInfo.h"
12+
#include "lldb/lldb-forward.h"
13+
#include "llvm/Testing/Support/Error.h"
1214

1315
#if LLDB_ENABLE_LIBEDIT
1416

@@ -25,7 +27,6 @@
2527
#include "TestingSupport/SubsystemRAII.h"
2628
#include "lldb/Host/Editline.h"
2729
#include "lldb/Host/FileSystem.h"
28-
#include "lldb/Host/Pipe.h"
2930
#include "lldb/Host/PseudoTerminal.h"
3031
#include "lldb/Host/StreamFile.h"
3132
#include "lldb/Utility/Status.h"
@@ -37,27 +38,6 @@ namespace {
3738
const size_t TIMEOUT_MILLIS = 5000;
3839
}
3940

40-
class FilePointer {
41-
public:
42-
FilePointer() = delete;
43-
44-
FilePointer(const FilePointer &) = delete;
45-
46-
FilePointer(FILE *file_p) : _file_p(file_p) {}
47-
48-
~FilePointer() {
49-
if (_file_p != nullptr) {
50-
const int close_result = fclose(_file_p);
51-
EXPECT_EQ(0, close_result);
52-
}
53-
}
54-
55-
operator FILE *() { return _file_p; }
56-
57-
private:
58-
FILE *_file_p;
59-
};
60-
6141
/**
6242
Wraps an Editline class, providing a simple way to feed
6343
input (as if from the keyboard) and receive output from Editline.
@@ -90,44 +70,39 @@ class EditlineAdapter {
9070
std::recursive_mutex output_mutex;
9171
std::unique_ptr<lldb_private::Editline> _editline_sp;
9272

93-
PseudoTerminal _pty;
94-
int _pty_primary_fd = -1;
95-
int _pty_secondary_fd = -1;
96-
97-
std::unique_ptr<FilePointer> _el_secondary_file;
73+
lldb::FileSP _el_primary_file;
74+
lldb::FileSP _el_secondary_file;
9875
};
9976

100-
EditlineAdapter::EditlineAdapter()
101-
: _editline_sp(), _pty(), _el_secondary_file() {
77+
EditlineAdapter::EditlineAdapter() : _editline_sp(), _el_secondary_file() {
10278
lldb_private::Status error;
79+
PseudoTerminal pty;
10380

10481
// Open the first primary pty available.
105-
EXPECT_THAT_ERROR(_pty.OpenFirstAvailablePrimary(O_RDWR), llvm::Succeeded());
82+
EXPECT_THAT_ERROR(pty.OpenFirstAvailablePrimary(O_RDWR), llvm::Succeeded());
83+
// Open the corresponding secondary pty.
84+
EXPECT_THAT_ERROR(pty.OpenSecondary(O_RDWR), llvm::Succeeded());
10685

10786
// Grab the primary fd. This is a file descriptor we will:
10887
// (1) write to when we want to send input to editline.
10988
// (2) read from when we want to see what editline sends back.
110-
_pty_primary_fd = _pty.GetPrimaryFileDescriptor();
89+
_el_primary_file.reset(
90+
new NativeFile(pty.ReleasePrimaryFileDescriptor(),
91+
lldb_private::NativeFile::eOpenOptionReadWrite, true));
11192

112-
// Open the corresponding secondary pty.
113-
EXPECT_THAT_ERROR(_pty.OpenSecondary(O_RDWR), llvm::Succeeded());
114-
_pty_secondary_fd = _pty.GetSecondaryFileDescriptor();
115-
116-
_el_secondary_file.reset(new FilePointer(fdopen(_pty_secondary_fd, "rw")));
117-
EXPECT_FALSE(nullptr == *_el_secondary_file);
118-
if (*_el_secondary_file == nullptr)
119-
return;
93+
_el_secondary_file.reset(
94+
new NativeFile(pty.ReleaseSecondaryFileDescriptor(),
95+
lldb_private::NativeFile::eOpenOptionReadWrite, true));
12096

12197
lldb::LockableStreamFileSP output_stream_sp =
122-
std::make_shared<LockableStreamFile>(*_el_secondary_file,
123-
NativeFile::Unowned, output_mutex);
98+
std::make_shared<LockableStreamFile>(_el_secondary_file, output_mutex);
12499
lldb::LockableStreamFileSP error_stream_sp =
125-
std::make_shared<LockableStreamFile>(*_el_secondary_file,
126-
NativeFile::Unowned, output_mutex);
100+
std::make_shared<LockableStreamFile>(_el_secondary_file, output_mutex);
127101

128102
// Create an Editline instance.
129103
_editline_sp.reset(new lldb_private::Editline(
130-
"gtest editor", *_el_secondary_file, output_stream_sp, error_stream_sp,
104+
"gtest editor", _el_secondary_file->GetStream(), output_stream_sp,
105+
error_stream_sp,
131106
/*color=*/false));
132107
_editline_sp->SetPrompt("> ");
133108

@@ -140,27 +115,22 @@ EditlineAdapter::EditlineAdapter()
140115

141116
void EditlineAdapter::CloseInput() {
142117
if (_el_secondary_file != nullptr)
143-
_el_secondary_file.reset(nullptr);
118+
_el_secondary_file->Close();
144119
}
145120

146121
bool EditlineAdapter::SendLine(const std::string &line) {
147122
// Ensure we're valid before proceeding.
148123
if (!IsValid())
149124
return false;
150125

126+
std::string out = line + "\n";
127+
151128
// Write the line out to the pipe connected to editline's input.
152-
ssize_t input_bytes_written =
153-
::write(_pty_primary_fd, line.c_str(),
154-
line.length() * sizeof(std::string::value_type));
155-
156-
const char *eoln = "\n";
157-
const size_t eoln_length = strlen(eoln);
158-
input_bytes_written =
159-
::write(_pty_primary_fd, eoln, eoln_length * sizeof(char));
160-
161-
EXPECT_NE(-1, input_bytes_written) << strerror(errno);
162-
EXPECT_EQ(eoln_length * sizeof(char), size_t(input_bytes_written));
163-
return eoln_length * sizeof(char) == size_t(input_bytes_written);
129+
size_t num_bytes = out.length() * sizeof(std::string::value_type);
130+
EXPECT_THAT_ERROR(_el_primary_file->Write(out.c_str(), num_bytes).takeError(),
131+
llvm::Succeeded());
132+
EXPECT_EQ(num_bytes, out.length() * sizeof(std::string::value_type));
133+
return true;
164134
}
165135

166136
bool EditlineAdapter::SendLines(const std::vector<std::string> &lines) {
@@ -215,7 +185,7 @@ bool EditlineAdapter::IsInputComplete(lldb_private::Editline *editline,
215185
}
216186

217187
void EditlineAdapter::ConsumeAllOutput() {
218-
FilePointer output_file(fdopen(_pty_primary_fd, "r"));
188+
FILE *output_file = _el_primary_file->GetStream();
219189

220190
int ch;
221191
while ((ch = fgetc(output_file)) != EOF) {

0 commit comments

Comments
 (0)