diff --git a/Editor.cc b/Editor.cc index 6623a87..419b6a4 100644 --- a/Editor.cc +++ b/Editor.cc @@ -169,21 +169,21 @@ Editor::AddBuffer(const Buffer &buf) // Buffer to a new address. Drain any in-flight swap records first so the // writer thread never dereferences an address that's about to move, then // rehome each attached buffer that actually moved. - if (swap_ && !bufs.empty()) - swap_->Flush(); + if (Swap() && !bufs.empty()) + Swap()->Flush(); std::vector old_addrs; old_addrs.reserve(bufs.size()); for (auto &b: bufs) old_addrs.push_back(&b); bufs.push_back(buf); - if (swap_) { + if (Swap()) { for (std::size_t i = 0; i < old_addrs.size(); ++i) { Buffer *new_addr = &bufs[i]; if (new_addr != old_addrs[i]) - bufs[i].SetSwapRecorder(swap_->Rehome(old_addrs[i], new_addr)); + bufs[i].SetSwapRecorder(Swap()->Rehome(old_addrs[i], new_addr)); } - swap_->Attach(&bufs.back()); - bufs.back().SetSwapRecorder(swap_->RecorderFor(&bufs.back())); + Swap()->Attach(&bufs.back()); + bufs.back().SetSwapRecorder(Swap()->RecorderFor(&bufs.back())); } if (bufs.size() == 1) { curbuf_ = 0; @@ -196,21 +196,21 @@ std::size_t Editor::AddBuffer(Buffer &&buf) { auto &bufs = Buffers(); - if (swap_ && !bufs.empty()) - swap_->Flush(); + if (Swap() && !bufs.empty()) + Swap()->Flush(); std::vector old_addrs; old_addrs.reserve(bufs.size()); for (auto &b: bufs) old_addrs.push_back(&b); bufs.push_back(std::move(buf)); - if (swap_) { + if (Swap()) { for (std::size_t i = 0; i < old_addrs.size(); ++i) { Buffer *new_addr = &bufs[i]; if (new_addr != old_addrs[i]) - bufs[i].SetSwapRecorder(swap_->Rehome(old_addrs[i], new_addr)); + bufs[i].SetSwapRecorder(Swap()->Rehome(old_addrs[i], new_addr)); } - swap_->Attach(&bufs.back()); - bufs.back().SetSwapRecorder(swap_->RecorderFor(&bufs.back())); + Swap()->Attach(&bufs.back()); + bufs.back().SetSwapRecorder(Swap()->RecorderFor(&bufs.back())); } if (bufs.size() == 1) { curbuf_ = 0; @@ -237,10 +237,10 @@ Editor::OpenFile(const std::string &path, std::string &err) if (!ok) return false; // Ensure swap recorder is attached for this buffer - if (swap_) { - swap_->Attach(&cur); - cur.SetSwapRecorder(swap_->RecorderFor(&cur)); - swap_->NotifyFilenameChanged(cur); + if (Swap()) { + Swap()->Attach(&cur); + cur.SetSwapRecorder(Swap()->RecorderFor(&cur)); + Swap()->NotifyFilenameChanged(cur); } // Setup highlighting using registry (extension + shebang) cur.EnsureHighlighter(); @@ -298,8 +298,8 @@ Editor::OpenFile(const std::string &path, std::string &err) } // Add as a new buffer and switch to it std::size_t idx = AddBuffer(std::move(b)); - if (swap_) { - swap_->NotifyFilenameChanged(Buffers()[idx]); + if (Swap()) { + Swap()->NotifyFilenameChanged(Buffers()[idx]); } SwitchTo(idx); // Defensive: ensure any active prompt is closed after a successful open @@ -514,26 +514,26 @@ Editor::CloseBuffer(std::size_t index) if (index >= bufs.size()) { return false; } - if (swap_) { + if (Swap()) { // Always remove swap file when closing a buffer on normal exit. // Swap files are for crash recovery; on clean close, we don't need them. // This prevents stale swap files from accumulating (e.g., when used as git editor). - swap_->Detach(&bufs[index], true); + Swap()->Detach(&bufs[index], true); bufs[index].SetSwapRecorder(nullptr); // Drain in-flight records before the erase-shift below moves other // buffers to new addresses (vector::erase never reallocates, but it does // move-assign each trailing buffer into the previous slot). - swap_->Flush(); + Swap()->Flush(); } bufs.erase(bufs.begin() + static_cast(index)); - if (swap_) { + if (Swap()) { // erase() shifts every buffer after `index` down by one slot in-place // (no reallocation), so the buffer now at slot i used to live at slot // i+1 (same underlying storage, since data() doesn't move on erase). for (std::size_t i = index; i < bufs.size(); ++i) { Buffer *new_addr = &bufs[i]; Buffer *old_addr = new_addr + 1; - bufs[i].SetSwapRecorder(swap_->Rehome(old_addr, new_addr)); + bufs[i].SetSwapRecorder(Swap()->Rehome(old_addr, new_addr)); } } if (bufs.empty()) { @@ -564,9 +564,9 @@ Editor::Reset() close_confirm_pending_ = false; close_after_save_ = false; auto &bufs = Buffers(); - if (swap_) { + if (Swap()) { for (auto &buf : bufs) - swap_->Detach(&buf, true); + Swap()->Detach(&buf, true); } bufs.clear(); curbuf_ = 0; diff --git a/Editor.h b/Editor.h index f367fff..b386b5e 100644 --- a/Editor.h +++ b/Editor.h @@ -607,9 +607,13 @@ public: // Share another editor's buffer list. When set, this editor operates on // the provided vector instead of its own. Pass nullptr to detach. - void SetSharedBuffers(std::vector *shared) + // Buffers hold recorders owned by the SwapManager that attached them, so + // editors sharing buffers must also share that manager (shared_swap); + // otherwise closing one window destroys recorders the other still uses. + void SetSharedBuffers(std::vector *shared, kte::SwapManager *shared_swap = nullptr) { shared_buffers_ = shared; + shared_swap_ = shared ? shared_swap : nullptr; curbuf_ = 0; } @@ -617,7 +621,7 @@ public: // Swap manager access (for advanced integrations/tests) [[nodiscard]] kte::SwapManager *Swap() { - return swap_.get(); + return shared_swap_ ? shared_swap_ : swap_.get(); } @@ -667,6 +671,7 @@ private: // Swap journaling manager (lifetime = editor) std::unique_ptr swap_; + kte::SwapManager *shared_swap_ = nullptr; // set with shared_buffers_; not owned // Kill ring (Emacs-like) std::vector kill_ring_; diff --git a/ImGuiFrontend.cc b/ImGuiFrontend.cc index cf6757d..65df2c0 100644 --- a/ImGuiFrontend.cc +++ b/ImGuiFrontend.cc @@ -378,7 +378,7 @@ GUIFrontend::OpenNewWindow_(Editor &primary) ws->height = h; // Secondary editor shares the primary's buffer list - ws->editor.SetSharedBuffers(&primary.Buffers()); + ws->editor.SetSharedBuffers(&primary.Buffers(), primary.Swap()); ws->editor.SetDimensions(primary.Rows(), primary.Cols()); // Open a new untitled buffer and switch to it in the new window. diff --git a/tests/test_audit_regressions.cc b/tests/test_audit_regressions.cc index 148d4c6..d37818d 100644 --- a/tests/test_audit_regressions.cc +++ b/tests/test_audit_regressions.cc @@ -11,6 +11,7 @@ #include "UndoTree.h" #include +#include #include #include @@ -225,3 +226,25 @@ TEST(Audit_D8_CircuitBreaker_WindowExpires) ASSERT_TRUE(cb.GetState() == kte::CircuitBreaker::State::Closed); ASSERT_EQ(cb.GetFailureCount(), (std::size_t) 1); } + + +// M3: an editor that shares another's buffers must attach them through the +// owner's SwapManager, so destroying it leaves no dangling recorders. +TEST(Audit_M3_SharedBuffers_UseOwnersSwapManager) +{ + TestHarness h; + Editor &primary = h.EditorRef(); + + auto secondary = std::make_unique(); + secondary->SetSharedBuffers(&primary.Buffers(), primary.Swap()); + ASSERT_TRUE(secondary->Swap() == primary.Swap()); + secondary->AddBuffer(Buffer()); + const std::size_t idx = primary.Buffers().size() - 1; + secondary.reset(); + + // Edits record through the buffer's swap recorder; before the fix it + // pointed into the destroyed secondary SwapManager. + Buffer &b = primary.Buffers()[idx]; + b.insert_text(0, 0, "abc"); + ASSERT_EQ(b.GetLineString(0), std::string("abc")); +}