Skip to content

Commit b824063

Browse files
committed
Fix crashes related to writing live output pgn/json files
The crahes are related to Tournament object accessing ChessGame object state, but these two objects are actually at different sides of slot and signal when a game is running. This translates to two different threads accessing the same state without protection. In practice, ChessGame may update the move list and board state while Tournament is reading the state in order to write the live pgn/json files. Fix this by moving the live file update from Tournament to ChessGame. This way the state is accessed only from a single thread.
1 parent 3226a9b commit b824063

File tree

4 files changed

+212
-197
lines changed

4 files changed

+212
-197
lines changed

projects/lib/src/chessgame.cpp

Lines changed: 201 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@
2727
#include "chessengine.h"
2828
#include "engineoption.h"
2929

30+
#include <jsonserializer.h>
31+
#include <QFileInfo>
32+
33+
3034
QString ChessGame::evalString(const MoveEvaluation& eval, const Chess::Move& move)
3135
{
3236
if (eval.isEmpty())
@@ -513,6 +517,8 @@ void ChessGame::onMoveMade(const Chess::Move& move)
513517
stop(false);
514518
emitLastMove();
515519
}
520+
521+
updateLiveFiles();
516522
}
517523

518524
void ChessGame::startTurn()
@@ -752,6 +758,15 @@ void ChessGame::setBookOwnership(bool enabled)
752758
m_bookOwnership = enabled;
753759
}
754760

761+
void ChessGame::setLiveOutput(const QString &livePgnOut, PgnGame::PgnMode livePgnOutMode,
762+
bool pgnFormat, bool jsonFormat)
763+
{
764+
m_livePgnOut = livePgnOut;
765+
m_livePgnOutMode = livePgnOutMode;
766+
m_pgnFormat = pgnFormat;
767+
m_jsonFormat = jsonFormat;
768+
}
769+
755770
void ChessGame::pauseThread()
756771
{
757772
m_pauseSem.release();
@@ -1000,7 +1015,9 @@ void ChessGame::startGame()
10001015
return;
10011016
}
10021017
}
1003-
1018+
1019+
updateLiveFiles();
1020+
10041021
for (int i = 0; i < 2; i++)
10051022
{
10061023
connect(m_player[i], SIGNAL(moveMade(Chess::Move)),
@@ -1012,3 +1029,186 @@ void ChessGame::startGame()
10121029

10131030
startTurn();
10141031
}
1032+
1033+
void ChessGame::updateLiveFiles() const
1034+
{
1035+
if (m_livePgnOut.isEmpty()) return;
1036+
1037+
const PgnGame* const pgn = m_pgn;
1038+
1039+
if (m_pgnFormat)
1040+
{
1041+
const QString fileName(m_livePgnOut + ".pgn");
1042+
QFile::resize(fileName, 0);
1043+
pgn->write(fileName, m_livePgnOutMode);
1044+
}
1045+
1046+
if (m_jsonFormat)
1047+
{
1048+
Chess::Board* board = m_board->copy();
1049+
board->setFenString(board->startingFenString());
1050+
1051+
QVariantMap pMap;
1052+
1053+
// Parse and assemble engine options
1054+
QStringList engines = pgn->initialComment().split(',', QString::SkipEmptyParts);
1055+
for (QString& engine : engines)
1056+
{
1057+
engine = engine.trimmed();
1058+
const int ePos = engine.indexOf(':');
1059+
if (ePos > 0)
1060+
{
1061+
QVariantList oList;
1062+
QStringList options = engine.mid(ePos + 1).trimmed().split(';', QString::SkipEmptyParts);
1063+
for (QString& option : options)
1064+
{
1065+
option = option.trimmed();
1066+
QVariantMap oMap;
1067+
const int oPos = option.indexOf('=');
1068+
if(oPos > 0)
1069+
{
1070+
oMap["Name"] = option.left(oPos).trimmed();
1071+
oMap["Value"] = option.mid(oPos + 1).trimmed();
1072+
} else
1073+
oMap["Name"] = option;
1074+
oList << oMap;
1075+
}
1076+
pMap[engine.left(ePos).trimmed()] = oList;
1077+
}
1078+
}
1079+
1080+
// Assemble tags
1081+
const QList< QPair<QString, QString> >& tags = pgn->tags();
1082+
QVariantMap hMap;
1083+
for(const QPair<QString, QString>& tagPair : tags)
1084+
hMap[tagPair.first] = tagPair.second;
1085+
pMap["Headers"] = hMap;
1086+
1087+
// Parse and assemble move stats
1088+
const QVector<PgnGame::MoveData>& moves = pgn->moves();
1089+
QVariantList mList;
1090+
for (const PgnGame::MoveData& move : moves)
1091+
{
1092+
QVariantMap mMap;
1093+
QVariantMap aMap;
1094+
1095+
mMap["m"] = move.moveString;
1096+
1097+
QString sq(static_cast<char>(move.move.sourceSquare().file() + 'a'));
1098+
sq += static_cast<char>(move.move.sourceSquare().rank() + '1');
1099+
mMap["from"] = sq;
1100+
1101+
sq = static_cast<char>(move.move.targetSquare().file() + 'a');
1102+
sq += static_cast<char>(move.move.targetSquare().rank() + '1');
1103+
mMap["to"] = sq;
1104+
1105+
mMap["book"] = false;
1106+
1107+
QStringList stats = move.comment.split(',', QString::SkipEmptyParts);
1108+
for(QString& stat : stats)
1109+
{
1110+
stat = stat.trimmed();
1111+
if (stat == "book") {
1112+
mMap["book"] = true;
1113+
} else {
1114+
const int pos = stat.indexOf('=');
1115+
if (pos > 0)
1116+
{
1117+
const QString name(stat.left(pos).trimmed());
1118+
const QString value(stat.mid(pos + 1).trimmed());
1119+
if (name == "pv")
1120+
{
1121+
QVariantMap pvMap;
1122+
QVariantList pvList;
1123+
1124+
pvMap["San"] = value;
1125+
1126+
int pvmCnt = 0;
1127+
QStringList pvMoves = value.split(' ', QString::SkipEmptyParts);
1128+
for (const QString& pvMoveStr : pvMoves)
1129+
{
1130+
QVariantMap pvMove;
1131+
1132+
const Chess::Move& pvbm(board->moveFromString(pvMoveStr));
1133+
if (pvbm.isNull())
1134+
break;
1135+
const Chess::GenericMove& gm(board->genericMove(pvbm));
1136+
1137+
board->makeMove(pvbm);
1138+
++pvmCnt;
1139+
1140+
pvMove["m"] = pvMoveStr;
1141+
pvMove["fen"] = board->fenString();
1142+
1143+
sq = static_cast<char>(gm.sourceSquare().file() + 'a');
1144+
sq += static_cast<char>(gm.sourceSquare().rank() + '1');
1145+
pvMove["from"] = sq;
1146+
1147+
sq = static_cast<char>(gm.targetSquare().file() + 'a');
1148+
sq += static_cast<char>(gm.targetSquare().rank() + '1');
1149+
pvMove["to"] = sq;
1150+
1151+
pvList << pvMove;
1152+
}
1153+
for(; pvmCnt > 0; --pvmCnt)
1154+
board->undoMove();
1155+
1156+
pvMap["Moves"] = pvList;
1157+
mMap["pv"] = pvMap;
1158+
}
1159+
else if (name == "mb")
1160+
{
1161+
QVariantMap mbMap;
1162+
int idx = 0;
1163+
for (const char* mstr : {"p", "n", "b", "r", "q"})
1164+
{
1165+
mbMap[mstr] = value.mid(idx, 2).toInt();
1166+
idx += 2;
1167+
}
1168+
mMap["material"] = mbMap;
1169+
}
1170+
else if (name == "R50")
1171+
aMap["FiftyMoves"] = value.toInt();
1172+
else if (name == "Rd")
1173+
aMap["Draw"] = value.toInt();
1174+
else if (name == "Rr")
1175+
aMap["ResignOrWin"] = value.toInt();
1176+
else
1177+
mMap[name] = value;
1178+
}
1179+
else // real comment
1180+
mMap["rem"] = stat;
1181+
}
1182+
}
1183+
if (!aMap.empty())
1184+
mMap["adjudication"] = aMap;
1185+
1186+
board->makeMove(board->moveFromGenericMove(move.move));
1187+
1188+
mMap["fen"] = board->fenString();
1189+
1190+
mList << mMap;
1191+
}
1192+
pMap["Moves"] = mList;
1193+
1194+
delete board;
1195+
1196+
const QString tempName(m_livePgnOut + "_temp.json");
1197+
const QString finalName(m_livePgnOut + ".json");
1198+
if (QFile::exists(tempName))
1199+
QFile::remove(tempName);
1200+
QFile output(tempName);
1201+
if (!output.open(QIODevice::WriteOnly | QIODevice::Text)) {
1202+
qWarning("cannot open live JSON output file: %s", qUtf8Printable(tempName));
1203+
} else {
1204+
QTextStream out(&output);
1205+
JsonSerializer serializer(pMap);
1206+
serializer.serialize(out);
1207+
output.close();
1208+
if (QFile::exists(finalName))
1209+
QFile::remove(finalName);
1210+
if (!QFile::rename(tempName, finalName))
1211+
qWarning("cannot rename live JSON output file: %s to %s", qUtf8Printable(tempName), qUtf8Printable(finalName));
1212+
}
1213+
}
1214+
}

projects/lib/src/chessgame.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ class LIB_EXPORT ChessGame : public QObject
7171
void setAdjudicator(const GameAdjudicator& adjudicator);
7272
void setStartDelay(int time);
7373
void setBookOwnership(bool enabled);
74+
void setLiveOutput(const QString &livePgnOut, PgnGame::PgnMode livePgnOutMode,
75+
bool pgnFormat, bool jsonFormat);
7476

7577
void generateOpening();
7678

@@ -123,6 +125,8 @@ class LIB_EXPORT ChessGame : public QObject
123125
void addPgnMove(const Chess::Move& move, const QString& comment);
124126
void emitLastMove();
125127

128+
void updateLiveFiles() const;
129+
126130
QString evalString(const MoveEvaluation& eval, const Chess::Move& move);
127131
QString statusString(const Chess::Move& move, bool doMove);
128132

@@ -147,6 +151,12 @@ class LIB_EXPORT ChessGame : public QObject
147151
QSemaphore m_pauseSem;
148152
QSemaphore m_resumeSem;
149153
GameAdjudicator m_adjudicator;
154+
155+
// live output support
156+
QString m_livePgnOut;
157+
PgnGame::PgnMode m_livePgnOutMode = PgnGame::Minimal;
158+
bool m_pgnFormat = false;
159+
bool m_jsonFormat = false;
150160
};
151161

152162
#endif // CHESSGAME_H

0 commit comments

Comments
 (0)