Skip to content

Commit b93a36e

Browse files
christianparpartchristianparpart
authored
Merge pull request #777 from contour-terminal/improvement/pty-deferred-start
defer PTY (and Process) creation
2 parents 797da8f + afdd751 commit b93a36e

15 files changed

+101
-54
lines changed

‎Changelog.md

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
- [Linux] Provide an AppStream XML file.
77
- [Linux] Drop KDE/KWin dependency on the binary by implementing enabling blur-behind background manually.
88
- [Linux] Adds support for blur-behind window on GNOME shell (Please read https://github.com/aunetx/blur-my-shell/issues/300 for further details if in trouble).
9+
- Changes behavior of PTY (and shell process) creation until only when a PTY is required by the terminal emulator during instanciation, possibly avoiding problems with xdotool running too early.
910
- Internal: Y-axis inverted to match GUI coordinate systems where (0, 0) is top left rather than bottom left.
1011
- Fixes logging file toggle.
1112

‎src/contour/TerminalSession.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ void TerminalSession::scheduleRedraw()
167167

168168
void TerminalSession::start()
169169
{
170+
terminal_.device().start();
170171
screenUpdateThread_ = make_unique<std::thread>(bind(&TerminalSession::mainLoop, this));
171172
}
172173

‎src/terminal/Process.h

+1
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ class [[nodiscard]] Process: public Pty
9595

9696
// Pty overrides
9797
// clang-format off
98+
void start() override;
9899
PtySlave& slave() noexcept override { return pty().slave(); }
99100
void close() override { pty().close(); }
100101
bool isClosed() const noexcept override { return pty().isClosed(); }

‎src/terminal/Process_unix.cpp

+23-13
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,13 @@ namespace
113113

114114
struct Process::Private
115115
{
116-
mutable pid_t pid {};
116+
string path;
117+
vector<string> args;
118+
FileSystem::path cwd;
119+
Environment env;
120+
117121
unique_ptr<Pty> pty {};
122+
mutable pid_t pid {};
118123
mutable std::mutex exitStatusMutex {};
119124
mutable std::optional<Process::ExitStatus> exitStatus {};
120125

@@ -126,10 +131,15 @@ Process::Process(string const& _path,
126131
FileSystem::path const& _cwd,
127132
Environment const& _env,
128133
unique_ptr<Pty> _pty):
129-
d(new Private {}, [](Private* p) { delete p; })
134+
d(new Private { _path, _args, _cwd, _env, move(_pty) }, [](Private* p) { delete p; })
130135
{
136+
}
137+
138+
void Process::start()
139+
{
140+
d->pty->start();
141+
131142
d->pid = fork();
132-
d->pty = move(_pty);
133143

134144
UnixPipe* stdoutFastPipe = [this]() -> UnixPipe* {
135145
if (auto* p = dynamic_cast<SystemPty*>(d->pty.get()))
@@ -150,42 +160,42 @@ Process::Process(string const& _path,
150160
{
151161
(void) d->pty->slave().login();
152162

153-
auto const& cwd = _cwd.generic_string();
163+
auto const& cwd = d->cwd.generic_string();
154164
if (!isFlatpak())
155165
{
156-
if (!_cwd.empty() && chdir(cwd.c_str()) != 0)
166+
if (!d->cwd.empty() && chdir(cwd.c_str()) != 0)
157167
{
158168
printf("Failed to chdir to \"%s\". %s\n", cwd.c_str(), strerror(errno));
159169
exit(EXIT_FAILURE);
160170
}
161171

162-
for (auto&& [name, value]: _env)
172+
for (auto&& [name, value]: d->env)
163173
setenv(name.c_str(), value.c_str(), true);
164174

165175
if (stdoutFastPipe)
166176
setenv(StdoutFastPipeEnvironmentName.data(), StdoutFastPipeFdStr.data(), true);
167177
}
168178

169-
char** argv = [=]() -> char** {
179+
char** argv = [stdoutFastPipe, this]() -> char** {
170180
if (!isFlatpak())
171-
return createArgv(_path, _args, 0);
181+
return createArgv(d->path, d->args, 0);
172182

173183
// Prepend flatpak to jump out of sandbox:
174184
// flatpak-spawn --host --watch-bus --env=TERM=$TERM /bin/zsh
175185
auto realArgs = std::vector<string> {};
176186
realArgs.emplace_back("--host");
177187
realArgs.emplace_back("--watch-bus");
178-
if (!_cwd.empty())
179-
realArgs.emplace_back(fmt::format("--directory={}", cwd));
188+
if (!d->cwd.empty())
189+
realArgs.emplace_back(fmt::format("--directory={}", d->cwd.generic_string()));
180190
if (auto const value = getenv("TERM"))
181191
realArgs.emplace_back(fmt::format("--env=TERM={}", value));
182-
for (auto&& [name, value]: _env)
192+
for (auto&& [name, value]: d->env)
183193
realArgs.emplace_back(fmt::format("--env={}={}", name, value));
184194
if (stdoutFastPipe)
185195
realArgs.emplace_back(
186196
fmt::format("--env={}={}", StdoutFastPipeEnvironmentName, StdoutFastPipeFd));
187-
realArgs.push_back(_path);
188-
for (auto const& arg: _args)
197+
realArgs.push_back(d->path);
198+
for (auto const& arg: d->args)
189199
realArgs.push_back(arg);
190200

191201
return createArgv("/usr/bin/flatpak-spawn", realArgs, 0);

‎src/terminal/pty/ConPty.cpp

+21-18
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,25 @@ ConPty::ConPty(PageSize const& _windowSize): size_ { _windowSize }
6969
master_ = INVALID_HANDLE_VALUE;
7070
input_ = INVALID_HANDLE_VALUE;
7171
output_ = INVALID_HANDLE_VALUE;
72+
buffer_.resize(10240);
73+
}
74+
75+
ConPty::~ConPty()
76+
{
77+
PtyLog()("~ConPty()");
78+
close();
79+
}
80+
81+
bool ConPty::isClosed() const noexcept
82+
{
83+
return master_ == INVALID_HANDLE_VALUE;
84+
}
85+
86+
void ConPty::start()
87+
{
88+
PtyLog()("Starting ConPTY");
89+
assert(!slave_);
90+
7291
slave_ = make_unique<ConPtySlave>(output_);
7392

7493
HANDLE hPipePTYIn { INVALID_HANDLE_VALUE };
@@ -85,11 +104,8 @@ ConPty::ConPty(PageSize const& _windowSize): size_ { _windowSize }
85104
}
86105

87106
// Create the Pseudo Console of the required size, attached to the PTY-end of the pipes
88-
HRESULT hr = CreatePseudoConsole({ unbox<SHORT>(_windowSize.columns), unbox<SHORT>(_windowSize.lines) },
89-
hPipePTYIn,
90-
hPipePTYOut,
91-
0,
92-
&master_);
107+
HRESULT hr = CreatePseudoConsole(
108+
{ unbox<SHORT>(size_.columns), unbox<SHORT>(size_.lines) }, hPipePTYIn, hPipePTYOut, 0, &master_);
93109

94110
if (hPipePTYIn != INVALID_HANDLE_VALUE)
95111
CloseHandle(hPipePTYIn);
@@ -99,19 +115,6 @@ ConPty::ConPty(PageSize const& _windowSize): size_ { _windowSize }
99115

100116
if (hr != S_OK)
101117
throw runtime_error { GetLastErrorAsString() };
102-
103-
buffer_.resize(10240);
104-
}
105-
106-
ConPty::~ConPty()
107-
{
108-
PtyLog()("~ConPty()");
109-
close();
110-
}
111-
112-
bool ConPty::isClosed() const noexcept
113-
{
114-
return master_ == INVALID_HANDLE_VALUE;
115118
}
116119

117120
void ConPty::close()

‎src/terminal/pty/ConPty.h

+1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ class ConPty: public Pty
3131
explicit ConPty(PageSize const& windowSize);
3232
~ConPty() override;
3333

34+
void start() override;
3435
void close() override;
3536
bool isClosed() const noexcept override;
3637

‎src/terminal/pty/LinuxPty.cpp

+8-5
Original file line numberDiff line numberDiff line change
@@ -163,14 +163,16 @@ int LinuxPty::Slave::write(std::string_view text) noexcept
163163
}
164164
// }}}
165165

166-
LinuxPty::LinuxPty(PageSize const& _windowSize, optional<ImageSize> _pixels):
167-
LinuxPty(createLinuxPty(_windowSize, _pixels), _windowSize)
166+
LinuxPty::LinuxPty(PageSize pageSize, optional<ImageSize> pixels): _pageSize { pageSize }, _pixels { pixels }
168167
{
169168
}
170169

171-
LinuxPty::LinuxPty(PtyHandles handles, PageSize pageSize):
172-
_masterFd { unbox<int>(handles.master) }, _pageSize { pageSize }, _slave { handles.slave }
170+
void LinuxPty::start()
173171
{
172+
auto const handles = createLinuxPty(_pageSize, _pixels);
173+
_masterFd = unbox<int>(handles.master);
174+
_slave = std::make_unique<Slave>(handles.slave);
175+
174176
if (!detail::setFileFlags(_masterFd, O_CLOEXEC | O_NONBLOCK))
175177
throw runtime_error { "Failed to configure PTY. "s + strerror(errno) };
176178

@@ -210,7 +212,8 @@ LinuxPty::~LinuxPty()
210212

211213
PtySlave& LinuxPty::slave() noexcept
212214
{
213-
return _slave;
215+
assert(_slave);
216+
return *_slave;
214217
}
215218

216219
PtyMasterHandle LinuxPty::handle() const noexcept

‎src/terminal/pty/LinuxPty.h

+8-6
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <terminal/pty/UnixPty.h> // UnixPipe (TODO: move somewhere else)
1818

1919
#include <array>
20+
#include <memory>
2021
#include <optional>
2122
#include <vector>
2223

@@ -49,13 +50,13 @@ class LinuxPty: public Pty
4950
PtySlaveHandle slave;
5051
};
5152

52-
LinuxPty(PageSize const& _windowSize, std::optional<ImageSize> _pixels);
53-
LinuxPty(PtyHandles handles, PageSize size);
53+
LinuxPty(PageSize _windowSize, std::optional<ImageSize> _pixels);
5454
~LinuxPty() override;
5555

5656
PtySlave& slave() noexcept override;
5757

5858
[[nodiscard]] PtyMasterHandle handle() const noexcept;
59+
void start() override;
5960
void close() override;
6061
[[nodiscard]] bool isClosed() const noexcept override;
6162
void wakeupReader() noexcept override;
@@ -72,12 +73,13 @@ class LinuxPty: public Pty
7273
std::optional<std::string_view> readSome(int fd, char* target, size_t n) noexcept;
7374
int waitForReadable(std::chrono::milliseconds timeout) noexcept;
7475

75-
int _masterFd;
76-
int _epollFd;
77-
int _eventFd;
76+
int _masterFd = -1;
77+
int _epollFd = -1;
78+
int _eventFd = -1;
7879
UnixPipe _stdoutFastPipe;
7980
PageSize _pageSize;
80-
Slave _slave;
81+
std::optional<ImageSize> _pixels;
82+
std::unique_ptr<Slave> _slave;
8183
};
8284

8385
} // namespace terminal

‎src/terminal/pty/MockPty.cpp

+5
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,11 @@ void MockPty::resizeScreen(PageSize _cells, std::optional<ImageSize> _pixels)
5656
pixelSize_ = _pixels;
5757
}
5858

59+
void MockPty::start()
60+
{
61+
closed_ = false;
62+
}
63+
5964
void MockPty::close()
6065
{
6166
closed_ = true;

‎src/terminal/pty/MockPty.h

+1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ class MockPty: public Pty
3636
[[nodiscard]] PageSize pageSize() const noexcept override;
3737
void resizeScreen(PageSize _cells, std::optional<ImageSize> _pixels = std::nullopt) override;
3838

39+
void start() override;
3940
void close() override;
4041
[[nodiscard]] bool isClosed() const noexcept override;
4142

‎src/terminal/pty/MockViewPty.cpp

+5
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,11 @@ void MockViewPty::resizeScreen(terminal::PageSize _cells, std::optional<terminal
6666
pixelSize_ = _pixels;
6767
}
6868

69+
void MockViewPty::start()
70+
{
71+
closed_ = false;
72+
}
73+
6974
void MockViewPty::close()
7075
{
7176
closed_ = true;

‎src/terminal/pty/MockViewPty.h

+1
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ class MockViewPty: public Pty
3535
[[nodiscard]] PageSize pageSize() const noexcept override;
3636
void resizeScreen(PageSize _cells, std::optional<ImageSize> _pixels = std::nullopt) override;
3737

38+
void start() override;
3839
void close() override;
3940
[[nodiscard]] bool isClosed() const noexcept override;
4041

‎src/terminal/pty/Pty.h

+3
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ class Pty
6666

6767
virtual ~Pty() = default;
6868

69+
/// Starts the PTY instance.
70+
virtual void start() = 0;
71+
6972
virtual PtySlave& slave() noexcept = 0;
7073

7174
/// Releases this PTY early.

‎src/terminal/pty/UnixPty.cpp

+15-9
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
#include <sys/wait.h>
4949

5050
using crispy::BufferObject;
51+
using std::make_unique;
5152
using std::max;
5253
using std::min;
5354
using std::nullopt;
@@ -291,18 +292,22 @@ int UnixPty::Slave::write(std::string_view text) noexcept
291292
}
292293
// }}}
293294

294-
UnixPty::UnixPty(PageSize const& _windowSize, optional<ImageSize> _pixels):
295-
UnixPty(createUnixPty(_windowSize, _pixels), _windowSize)
295+
UnixPty::UnixPty(PageSize pageSize, optional<ImageSize> pixels):
296+
// clang-format off
297+
_masterFd { -1 },
298+
_pipe { -1, -1 },
299+
_pageSize { pageSize },
300+
_pixels { pixels },
301+
_slave { }
296302
{
297303
}
298304

299-
UnixPty::UnixPty(PtyHandles handles, PageSize pageSize):
300-
// clang-format off
301-
_masterFd { unbox<int>(handles.master) },
302-
_pipe {},
303-
_pageSize { pageSize },
304-
_slave { handles.slave }
305+
void UnixPty::start()
305306
{
307+
auto const handles = createUnixPty(_pageSize, _pixels);
308+
_masterFd = unbox<int>(handles.master);
309+
_slave = make_unique<Slave>(handles.slave);
310+
306311
// clang-format on
307312
if (!detail::setFileFlags(_masterFd, O_CLOEXEC | O_NONBLOCK))
308313
throw runtime_error { "Failed to configure PTY. "s + strerror(errno) };
@@ -332,7 +337,8 @@ UnixPty::~UnixPty()
332337

333338
PtySlave& UnixPty::slave() noexcept
334339
{
335-
return _slave;
340+
assert(started());
341+
return *_slave;
336342
}
337343

338344
PtyMasterHandle UnixPty::handle() const noexcept

‎src/terminal/pty/UnixPty.h

+7-3
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <terminal/pty/Pty.h>
1717

1818
#include <array>
19+
#include <memory>
1920
#include <optional>
2021
#include <vector>
2122

@@ -74,13 +75,13 @@ class UnixPty: public Pty
7475
PtySlaveHandle slave;
7576
};
7677

77-
UnixPty(PageSize const& _windowSize, std::optional<ImageSize> _pixels);
78-
UnixPty(PtyHandles handles, PageSize size);
78+
UnixPty(PageSize _windowSize, std::optional<ImageSize> _pixels);
7979
~UnixPty() override;
8080

8181
PtySlave& slave() noexcept override;
8282

8383
PtyMasterHandle handle() const noexcept;
84+
void start() override;
8485
void close() override;
8586
bool isClosed() const noexcept override;
8687
void wakeupReader() noexcept override;
@@ -96,11 +97,14 @@ class UnixPty: public Pty
9697
private:
9798
std::optional<std::string_view> readSome(int fd, char* target, size_t n) noexcept;
9899

100+
[[nodiscard]] bool started() const noexcept { return _masterFd != -1; }
101+
99102
int _masterFd;
100103
std::array<int, 2> _pipe;
101104
UnixPipe _stdoutFastPipe;
102105
PageSize _pageSize;
103-
Slave _slave;
106+
std::optional<ImageSize> _pixels;
107+
std::unique_ptr<Slave> _slave;
104108
};
105109

106110
} // namespace terminal

0 commit comments

Comments
 (0)