Skip to content

Fix PMS atomic persistence and truthful save responses #5666

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jensenpat merged 2 commits into from
Sep 16, 2026
Merged

Fix PMS atomic persistence and truthful save responses #5666

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
302c668
Fix PMS atomic persistence and failure reporting. Principle XIV.
rfoust Sep 13, 2026
daf401d
Silence PMS destructor activity on save failure. Principle XIV.
jensenpat Sep 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
161 changes: 121 additions & 40 deletions src/core/pms/PmsMailbox.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "core/pms/PmsMailbox.h"

#include "core/AppSettings.h"
#include "core/LogManager.h"
#include "core/tnc/Ax25Connection.h"

#include <QDir>
Expand All @@ -9,10 +10,13 @@
#include <QJsonArray>
#include <QJsonDocument>
#include <QJsonObject>
#include <QSaveFile>
#include <QSignalBlocker>
#include <QStorageInfo>
#include <QTimer>

#include <algorithm>
#include <utility>

namespace AetherSDR {

Expand All @@ -29,6 +33,30 @@ bool sameCall(const QString& a, const QString& b)
return a.compare(b, Qt::CaseInsensitive) == 0;
}

bool writeJsonAtomically(const QString& path, const QJsonObject& root, QString* error)
{
QSaveFile file(path);
// Never fall back to writing the target directly: a failed replacement must
// leave the last complete mailbox snapshot available to the next startup.
file.setDirectWriteFallback(false);
Comment thread
jensenpat marked this conversation as resolved.
if (!file.open(QIODevice::WriteOnly | QIODevice::Truncate)) {
*error = file.errorString();
return false;
}

const QByteArray bytes = QJsonDocument(root).toJson();
if (file.write(bytes) != bytes.size()) {
*error = file.errorString();
file.cancelWriting();
return false;
}
if (!file.commit()) {
*error = file.errorString();
return false;
}
return true;
}

} // namespace

PmsMailbox::PmsMailbox(QObject* parent)
Expand Down Expand Up @@ -72,8 +100,12 @@ PmsMailbox::PmsMailbox(QObject* parent)

PmsMailbox::~PmsMailbox()
{
if (m_loaded)
saveHeard();
if (m_loaded) {
// Parent dialog slots (appendSystemLine) are already gone when QWidget
// deletes this child. Keep the qCWarning; do not emit activity.
const QSignalBlocker blocker(this);
saveHeard(m_heard);
Comment thread
jensenpat marked this conversation as resolved.
Comment thread
jensenpat marked this conversation as resolved.
}
}
Comment thread
jensenpat marked this conversation as resolved.

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -108,7 +140,7 @@ void PmsMailbox::setEnabled(bool on)
} else {
m_link->reset();
m_beaconTimer->stop();
saveHeard();
saveHeard(m_heard);
emit activity(QStringLiteral("PMS disabled."));
}
emit stateChanged();
Expand Down Expand Up @@ -369,7 +401,7 @@ void PmsMailbox::recordHeard(const Frame& frame)
[](const Heard& a, const Heard& b) { return a.utc > b.utc; });
m_heard.resize(200);
}
saveHeard();
saveHeard(m_heard);
emit stateChanged();
}
}
Expand All @@ -380,9 +412,10 @@ void PmsMailbox::recordCaller(const Address& peer)
c.call = peer.toString();
c.utc = QDateTime::currentDateTimeUtc();
m_callers.append(c);
if (m_callers.size() > 500)
if (m_callers.size() > 500) {
m_callers.remove(0, m_callers.size() - 500);
saveCallers();
}
saveCallers(m_callers);
}

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -416,7 +449,7 @@ void PmsMailbox::onLinkDisconnected(const Address& peer, bool byPeer)
m_draftLines.clear();
if (m_sessionIdleTimer)
m_sessionIdleTimer->stop();
saveHeard();
saveHeard(m_heard);
emit stateChanged();
}

Expand Down Expand Up @@ -635,7 +668,8 @@ void PmsMailbox::cmdRead(const QString& args)
reply(QStringLiteral("USAGE: R <message-number>"));
return;
}
for (Message& m : m_messages) {
for (int i = 0; i < m_messages.size(); ++i) {
const Message& m = m_messages.at(i);
if (m.id != n)
continue;
if (!callerMayAccess(m)) {
Expand All @@ -650,8 +684,13 @@ void PmsMailbox::cmdRead(const QString& args)
reply(m.body.isEmpty() ? QStringLiteral("(no text)") : m.body);
reply(QStringLiteral("---"));
if (!m.read && (sameCall(m.to, m_caller.toString()) || sameCall(m.to, m_caller.call))) {
m.read = true;
saveMessages();
QVector<Message> updated = m_messages;
updated[i].read = true;
if (saveMessages(updated, m_nextId)) {
m_messages = std::move(updated);
} else {
reply(QStringLiteral("*** Read state was not saved; message remains unread."));
}
}
return;
}
Expand Down Expand Up @@ -679,8 +718,13 @@ void PmsMailbox::cmdKill(const QString& args)
reply(QStringLiteral("Not authorized to kill message %1.").arg(n));
return;
}
m_messages.remove(i);
saveMessages();
QVector<Message> updated = m_messages;
updated.remove(i);
if (!saveMessages(updated, m_nextId)) {
reply(QStringLiteral("*** Message %1 was not killed; storage error.").arg(n));
return;
}
m_messages = std::move(updated);
reply(QStringLiteral("Message %1 killed.").arg(n));
emit stateChanged();
return;
Expand Down Expand Up @@ -721,13 +765,21 @@ void PmsMailbox::cmdSendBegin(const QString& args, QChar type)
void PmsMailbox::finishCompose(bool save)
{
if (save) {
m_draft.id = m_nextId++;
m_draft.utc = QDateTime::currentDateTimeUtc();
m_draft.body = m_draftLines.join(QStringLiteral("\n"));
m_draft.read = false;
m_messages.append(m_draft);
saveMessages();
reply(QStringLiteral("MESSAGE %1 SAVED.").arg(m_draft.id));
Message candidate = m_draft;
candidate.id = m_nextId;
candidate.utc = QDateTime::currentDateTimeUtc();
candidate.body = m_draftLines.join(QStringLiteral("\n"));
candidate.read = false;
QVector<Message> updated = m_messages;
updated.append(candidate);
if (!saveMessages(updated, m_nextId + 1)) {
reply(QStringLiteral("*** MESSAGE NOT SAVED; storage error. Draft retained: send /EX to retry."));
Comment thread
jensenpat marked this conversation as resolved.
return;
}
m_messages = std::move(updated);
m_nextId += 1;
m_draft = candidate;
reply(QStringLiteral("MESSAGE %1 SAVED.").arg(candidate.id));
emit stateChanged();
} else {
reply(QStringLiteral("Message aborted."));
Expand Down Expand Up @@ -805,9 +857,9 @@ QString PmsMailbox::messagesPath() const { return storageDir() + QStringLiteral(
QString PmsMailbox::callersPath() const { return storageDir() + QStringLiteral("/callers.json"); }
QString PmsMailbox::heardPath() const { return storageDir() + QStringLiteral("/heard.json"); }

void PmsMailbox::ensureStorageDir() const
bool PmsMailbox::ensureStorageDir() const
{
QDir().mkpath(storageDir());
return QDir().mkpath(storageDir());
}

void PmsMailbox::loadAll()
Expand Down Expand Up @@ -869,11 +921,23 @@ void PmsMailbox::loadAll()
}
}

void PmsMailbox::saveMessages() const
void PmsMailbox::reportPersistenceFailure(const QString& store, const QString& detail)
{
const QString message = QStringLiteral("PMS could not save %1: %2")
.arg(store, detail);
qCWarning(lcAx25).noquote() << message;
emit activity(message);
}

bool PmsMailbox::saveMessages(const QVector<Message>& messages, int nextId)
{
ensureStorageDir();
if (!ensureStorageDir()) {
reportPersistenceFailure(QStringLiteral("messages"), QStringLiteral("could not create %1")
.arg(storageDir()));
return false;
}
QJsonArray arr;
for (const Message& m : m_messages) {
for (const Message& m : messages) {
QJsonObject o;
o.insert(QStringLiteral("id"), m.id);
o.insert(QStringLiteral("type"), QString(m.type));
Expand All @@ -886,35 +950,49 @@ void PmsMailbox::saveMessages() const
arr.append(o);
}
QJsonObject root;
root.insert(QStringLiteral("nextId"), m_nextId);
root.insert(QStringLiteral("nextId"), nextId);
root.insert(QStringLiteral("messages"), arr);
QFile f(messagesPath());
if (f.open(QIODevice::WriteOnly | QIODevice::Truncate))
f.write(QJsonDocument(root).toJson());
QString error;
if (!writeJsonAtomically(messagesPath(), root, &error)) {
reportPersistenceFailure(QStringLiteral("messages"), error);
return false;
}
return true;
}

void PmsMailbox::saveCallers() const
bool PmsMailbox::saveCallers(const QVector<Caller>& callers)
{
ensureStorageDir();
if (!ensureStorageDir()) {
reportPersistenceFailure(QStringLiteral("callers"), QStringLiteral("could not create %1")
.arg(storageDir()));
return false;
}
QJsonArray arr;
for (const Caller& c : m_callers) {
for (const Caller& c : callers) {
QJsonObject o;
o.insert(QStringLiteral("call"), c.call);
o.insert(QStringLiteral("utc"), c.utc.toString(Qt::ISODate));
arr.append(o);
}
QJsonObject root;
root.insert(QStringLiteral("callers"), arr);
QFile f(callersPath());
if (f.open(QIODevice::WriteOnly | QIODevice::Truncate))
f.write(QJsonDocument(root).toJson());
QString error;
if (!writeJsonAtomically(callersPath(), root, &error)) {
reportPersistenceFailure(QStringLiteral("callers"), error);
return false;
}
return true;
}

void PmsMailbox::saveHeard() const
bool PmsMailbox::saveHeard(const QVector<Heard>& heard)
{
ensureStorageDir();
if (!ensureStorageDir()) {
reportPersistenceFailure(QStringLiteral("heard stations"), QStringLiteral("could not create %1")
.arg(storageDir()));
return false;
}
QJsonArray arr;
for (const Heard& h : m_heard) {
for (const Heard& h : heard) {
QJsonObject o;
o.insert(QStringLiteral("call"), h.call);
o.insert(QStringLiteral("dest"), h.dest);
Expand All @@ -925,9 +1003,12 @@ void PmsMailbox::saveHeard() const
}
QJsonObject root;
root.insert(QStringLiteral("heard"), arr);
QFile f(heardPath());
if (f.open(QIODevice::WriteOnly | QIODevice::Truncate))
f.write(QJsonDocument(root).toJson());
QString error;
if (!writeJsonAtomically(heardPath(), root, &error)) {
reportPersistenceFailure(QStringLiteral("heard stations"), error);
return false;
}
return true;
}

} // namespace AetherSDR
9 changes: 5 additions & 4 deletions src/core/pms/PmsMailbox.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,12 @@ public slots:
QString messagesPath() const;
QString callersPath() const;
QString heardPath() const;
void ensureStorageDir() const;
bool ensureStorageDir() const;
void loadAll();
void saveMessages() const;
void saveCallers() const;
void saveHeard() const;
bool saveMessages(const QVector<Message>& messages, int nextId);
bool saveCallers(const QVector<Caller>& callers);
bool saveHeard(const QVector<Heard>& heard);
void reportPersistenceFailure(const QString& store, const QString& detail);

// Cap on unterminated inbound text. paclen tops out at 256 bytes and a
// mailbox command is a few dozen characters, so anything approaching this
Expand Down
Loading
Loading