Load/save robustness: OOM-safe load, guarded open loop, save fallbacks
- OpenFromFile builds the new content in a separate PieceTable and swaps it in only when complete. A reload that ran out of memory used to leave the buffer empty (filename, journal and dirty state intact), and the next save truncated the file. - Editor::ProcessPendingOpens catches exceptions from opening (it runs in every frontend's loop, outside command dispatch, so bad_alloc on a huge file ended the editor); the terminal draw is guarded too. - The hard-link save copy uses a unique mkstemp name, so a leftover copy no longer blocks every later save; if no copy can be created the file is still written in place. - When no temp file can be created in the directory (file writable, directory not), an existing regular file is written in place instead of the save failing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rcVQiiWyTzoNrsvLSaAfB
This commit is contained in:
@@ -170,19 +170,20 @@ write_in_place(const std::string &path, const char *data, std::size_t len, std::
|
||||
}
|
||||
|
||||
|
||||
// Create `path` (which must not exist) with the content, fsynced.
|
||||
// Create a new file named from `tmpl` (a mkstemp template, which receives
|
||||
// the chosen name) holding the content, fsynced.
|
||||
static bool
|
||||
write_new_file(const std::string &path, const char *data, std::size_t len, std::string &err)
|
||||
write_new_file(std::string &tmpl, const char *data, std::size_t len, std::string &err)
|
||||
{
|
||||
int flags = O_WRONLY | O_CREAT | O_EXCL;
|
||||
#ifdef O_CLOEXEC
|
||||
flags |= O_CLOEXEC;
|
||||
#endif
|
||||
const int fd = kte::syscall::Open(path.c_str(), flags, 0600);
|
||||
std::vector<char> name(tmpl.begin(), tmpl.end());
|
||||
name.push_back('\0');
|
||||
const int fd = kte::syscall::Mkstemp(name.data());
|
||||
if (fd < 0) {
|
||||
err = "Failed to create " + path + ": " + std::strerror(errno);
|
||||
err = "Failed to create " + tmpl + ": " + std::strerror(errno);
|
||||
return false;
|
||||
}
|
||||
tmpl.assign(name.data());
|
||||
const std::string &path = tmpl;
|
||||
bool ok = write_all_fd(fd, data, len, err);
|
||||
if (ok && kte::syscall::Fsync(fd) != 0) {
|
||||
err = std::string("fsync failed: ") + std::strerror(errno);
|
||||
@@ -220,14 +221,18 @@ atomic_write_file(const std::string &path_in, const char *data, std::size_t len,
|
||||
// names would keep the old content.
|
||||
if (dst_exists && S_ISREG(dst_st.st_mode) && dst_st.st_nlink > 1) {
|
||||
// Truncating in place risks the only copy if the write then fails
|
||||
// (ENOSPC, EIO, crash), so first write and sync a copy beside it.
|
||||
const std::string copy = path + ".kte-save";
|
||||
if (!write_new_file(copy, data, len, err))
|
||||
return false;
|
||||
// (ENOSPC, EIO, crash), so first write and sync a copy beside it. If
|
||||
// no copy can be made (directory not writable, name too long), write
|
||||
// in place anyway rather than refuse to save.
|
||||
std::string copy = path + ".kte-save.XXXXXX";
|
||||
std::string copy_err;
|
||||
const bool have_copy = write_new_file(copy, data, len, copy_err);
|
||||
if (!write_in_place(path, data, len, err)) {
|
||||
if (have_copy)
|
||||
err += " (the new content is in " + copy + ")";
|
||||
return false;
|
||||
}
|
||||
if (have_copy)
|
||||
(void) ::unlink(copy.c_str());
|
||||
best_effort_fsync_dir(path);
|
||||
return true;
|
||||
@@ -255,8 +260,16 @@ atomic_write_file(const std::string &path_in, const char *data, std::size_t len,
|
||||
};
|
||||
|
||||
if (!kte::RetryOnTransientError(mkstemp_fn, kte::RetryPolicy::Aggressive(), err)) {
|
||||
const int saved_errno = errno;
|
||||
// No temp file in the directory (e.g. the file is writable but its
|
||||
// directory is not): an existing regular file can still be written in
|
||||
// place, as editors traditionally do.
|
||||
if (fd < 0 && dst_exists && S_ISREG(dst_st.st_mode)) {
|
||||
err.clear();
|
||||
return write_in_place(path, data, len, err);
|
||||
}
|
||||
if (fd < 0) {
|
||||
err = std::string("Failed to create temp file for save: ") + std::strerror(errno) + err;
|
||||
err = std::string("Failed to create temp file for save: ") + std::strerror(saved_errno) + err;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
@@ -618,9 +631,20 @@ Buffer::OpenFromFile(const std::string &path, std::string &err)
|
||||
return false;
|
||||
}
|
||||
(void) kte::syscall::Close(fd);
|
||||
content_.Clear();
|
||||
// Build the new content aside and swap it in only once complete: if the
|
||||
// copy runs out of memory, the buffer keeps its old text (clearing first
|
||||
// left an empty buffer that a later save wrote over the file).
|
||||
PieceTable fresh;
|
||||
try {
|
||||
if (!data.empty())
|
||||
content_.Append(data.data(), data.size());
|
||||
fresh.Append(data.data(), data.size());
|
||||
std::string().swap(data);
|
||||
} catch (const std::bad_alloc &) {
|
||||
err = "File too large to load: " + norm;
|
||||
kte::ErrorHandler::Instance().Error("Buffer", err, norm);
|
||||
return false;
|
||||
}
|
||||
content_ = std::move(fresh);
|
||||
rows_cache_dirty_ = true;
|
||||
nrows_ = 0; // not used under PieceTable
|
||||
filename_ = norm;
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
#include <utility>
|
||||
|
||||
#include "Editor.h"
|
||||
#include "ErrorHandler.h"
|
||||
#include "syntax/HighlighterRegistry.h"
|
||||
#include "syntax/CppHighlighter.h"
|
||||
#include "syntax/NullHighlighter.h"
|
||||
@@ -487,6 +488,24 @@ Editor::ResolveRecoveryPrompt(const bool yes)
|
||||
|
||||
bool
|
||||
Editor::ProcessPendingOpens()
|
||||
{
|
||||
// Opening can throw (bad_alloc on a huge file, filesystem errors). This
|
||||
// runs from every frontend's loop, outside command dispatch: report the
|
||||
// failure instead of letting it end the editor.
|
||||
try {
|
||||
return process_pending_opens_();
|
||||
} catch (const std::exception &e) {
|
||||
kte::ErrorHandler::Instance().Error("Editor", std::string("open failed: ") + e.what(), "");
|
||||
SetStatus(std::string("Open failed: ") + e.what());
|
||||
} catch (...) {
|
||||
SetStatus("Open failed");
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
|
||||
bool
|
||||
Editor::process_pending_opens_()
|
||||
{
|
||||
if (PromptActive())
|
||||
return false;
|
||||
|
||||
@@ -568,6 +568,11 @@ public:
|
||||
// Returns true if a file was opened during this call.
|
||||
bool ProcessPendingOpens();
|
||||
|
||||
private:
|
||||
bool process_pending_opens_();
|
||||
|
||||
public:
|
||||
|
||||
[[nodiscard]] bool HasPendingOpens() const;
|
||||
|
||||
// Swap recovery confirmation state. When non-None, a `PromptKind::Confirm`
|
||||
|
||||
@@ -135,7 +135,11 @@ TerminalFrontend::Step(Editor &ed, bool &running)
|
||||
// busy on large or long-line files while idle.
|
||||
const auto now = std::chrono::steady_clock::now();
|
||||
if (changed || now - last_draw_ >= std::chrono::milliseconds(1000)) {
|
||||
try {
|
||||
renderer_.Draw(ed);
|
||||
} catch (const std::exception &e) {
|
||||
ed.SetStatus(std::string("Display error: ") + e.what());
|
||||
}
|
||||
last_draw_ = now;
|
||||
first_frame_ = false;
|
||||
}
|
||||
|
||||
@@ -1110,3 +1110,19 @@ TEST(Audit_VisualLine_StaysInsideBuffer)
|
||||
(void) h.Undo();
|
||||
ASSERT_EQ(h.Text(), orig);
|
||||
}
|
||||
|
||||
|
||||
// A leftover copy from an interrupted hard-link save does not block saving.
|
||||
TEST(Audit_Save_HardLink_LeftoverCopyDoesNotBlock)
|
||||
{
|
||||
TempDir d("save_leftover");
|
||||
std::ofstream(d.path / "h.txt") << "old\n";
|
||||
std::filesystem::create_hard_link(d.path / "h.txt", d.path / "h2.txt");
|
||||
std::ofstream(d.path / "h.txt.kte-save") << "stale\n";
|
||||
Buffer b;
|
||||
std::string err;
|
||||
ASSERT_TRUE(b.OpenFromFile((d.path / "h.txt").string(), err));
|
||||
b.insert_text(0, 0, "new ");
|
||||
ASSERT_TRUE(b.Save(err));
|
||||
ASSERT_EQ(slurp(d.path / "h2.txt"), std::string("new old\n"));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user