Prevent changing Client's buffer while locked

If the current buffer is locked, it means we are in the middle of
creating the client or already changing the buffer, error out in this
case instead of getting into an inconsistent state.

Fixes #5338

5 files changed, 11 insertions, 5 deletions

diff --git a/src/client.cc b/src/client.cc
index 2cb1c973..14b7b7bc 100644
--- a/src/client.cc
+++ b/src/client.cc
@@ -194,6 +194,9 @@ void Client::change_buffer(Buffer& buffer, Optional<FunctionRef<void()>> set_sel
     if (m_buffer_reload_dialog_opened)
         close_buffer_reload_dialog();
 
+    if (context().buffer().flags() & Buffer::Flags::Locked)
+        throw runtime_error("Changing buffer is not allowed while current buffer is locked");
+
     buffer.flags() |= Buffer::Flags::Locked;
     OnScopeEnd unlock{[&] { buffer.flags() &= ~Buffer::Flags::Locked; }};
 
diff --git a/src/client_manager.cc b/src/client_manager.cc
index 2e7d76e7..6798864e 100644
--- a/src/client_manager.cc
+++ b/src/client_manager.cc
@@ -67,17 +67,18 @@ Client* ClientManager::create_client(std::unique_ptr<UserInterface>&& ui, int pi
     m_clients.emplace_back(client);
 
     auto& context = client->context();
-    if (buffer->name() == "*scratch*")
+    if (context.buffer().name() == "*scratch*")
         context.print_status({"This *scratch* buffer won't be automatically saved",
                               context.faces()["Information"]});
 
     if (init_coord)
     {
-        auto& selections = context.selections_write_only();
-        selections = SelectionList(*buffer, buffer->clamp(*init_coord));
+        context.selections_write_only() = SelectionList(*buffer, context.buffer().clamp(*init_coord));
         context.window().center_line(init_coord->line);
     }
 
+    auto(std::move(unlock)); // unlock now
+
     try
     {
         context.hooks().run_hook(Hook::ClientCreate, context.name(), context);
@@ -86,8 +87,7 @@ Client* ClientManager::create_client(std::unique_ptr<UserInterface>&& ui, int pi
     catch (Kakoune::runtime_error& error)
     {
         context.print_status({error.what().str(), context.faces()["Error"]});
-        context.hooks().run_hook(Hook::RuntimeError, error.what(),
-                                           context);
+        context.hooks().run_hook(Hook::RuntimeError, error.what(), context);
     }
 
     // Do not return the client if it already got moved to the trash
diff --git a/test/regression/5338-crash-when-changing-buffer-on-WinDiplay/cmd b/test/regression/5338-crash-when-changing-buffer-on-WinDiplay/cmd
new file mode 100644
index 00000000..a1a5692c
--- /dev/null
+++ b/test/regression/5338-crash-when-changing-buffer-on-WinDiplay/cmd
@@ -0,0 +1 @@
+:e -scratch<ret><c-o>
diff --git a/test/regression/5338-crash-when-changing-buffer-on-WinDiplay/in b/test/regression/5338-crash-when-changing-buffer-on-WinDiplay/in
new file mode 100644
index 00000000..8b137891
--- /dev/null
+++ b/test/regression/5338-crash-when-changing-buffer-on-WinDiplay/in
@@ -0,0 +1 @@
+
diff --git a/test/regression/5338-crash-when-changing-buffer-on-WinDiplay/rc b/test/regression/5338-crash-when-changing-buffer-on-WinDiplay/rc
new file mode 100644
index 00000000..6240a36f
--- /dev/null
+++ b/test/regression/5338-crash-when-changing-buffer-on-WinDiplay/rc
@@ -0,0 +1 @@
+hook global WinDisplay .* %{ edit out }