Fix varoius build action related problems

* Fix crashes in some situations
* Fix aborting reloading library dependencies
This commit is contained in:
Martchus 2022-02-22 22:49:40 +01:00
parent 9e15129d9d
commit 382169ab6e
7 changed files with 70 additions and 51 deletions

View File

@ -1,3 +1,4 @@
#include "./buildactionprivate.h"
#include "../webapi/session.h"
@ -167,7 +168,7 @@ const std::string &InternalBuildAction::findSetting(const std::string_view &sett
void InternalBuildAction::reportError(std::string &&error)
{
const auto buildActionLock = m_setup.building.lockToWrite();
m_buildAction->resultData = move(error);
m_buildAction->resultData = std::move(error);
m_buildAction->conclude(BuildActionResult::Failure);
}
@ -228,10 +229,10 @@ BuildAction &BuildAction::operator=(BuildAction &&other)
started = other.started;
finished = other.finished;
startAfter = std::move(other.startAfter);
m_log = std::move(other.m_log);
m_log = LogContext(this);
m_setup = other.m_setup;
m_aborted = false;
m_stopHandler = std::function<void(void)>();
m_stopHandler = std::bind(&BuildAction::terminateOngoingBuildProcesses, this);
m_concludeHandler = std::function<void(void)>();
m_ongoingProcesses.clear();
m_outputSession.reset();
@ -360,11 +361,6 @@ LibPkg::StorageID BuildAction::conclude(BuildActionResult result)
this->result = result;
finished = DateTime::gmtNow();
// tell clients waiting for output that it's over
if (const auto outputStreamingLock = std::unique_lock<std::mutex>(m_outputSessionMutex); m_outputSession) {
m_outputSession->writeEnd();
}
// start globally visible follow-up actions if succeeded
if (result == BuildActionResult::Success && m_setup) {
const auto followUps = m_setup->building.followUpBuildActions(id);
@ -376,6 +372,15 @@ LibPkg::StorageID BuildAction::conclude(BuildActionResult result)
// note: Not cleaning up the follow-up actions here because at some point I might implement recursive restarting.
}
// detatch build process sessions
if (const auto lock = std::unique_lock(m_outputSessionMutex); m_outputSession) {
m_outputSession->writeEnd(); // tell clients waiting for output that it's over
m_outputSession.reset();
}
if (const auto lock = std::unique_lock(m_processesMutex)) {
m_ongoingProcesses.clear();
}
// write build action to persistent storage
// TODO: should this also be done in the middle of the execution to have some "save points"?
auto id = LibPkg::StorageID();

View File

@ -180,6 +180,7 @@ public:
bool hasSucceeded() const;
static bool haveSucceeded(const std::vector<std::shared_ptr<BuildAction>> &buildActions);
bool isAborted() const;
const std::atomic_bool &aborted() const;
LibPkg::StorageID start(ServiceSetup &setup);
void assignStartAfter(const std::vector<std::shared_ptr<BuildAction>> &startsAfterBuildActions);
void abort();
@ -267,6 +268,11 @@ inline bool BuildAction::isAborted() const
return m_aborted.load();
}
inline const std::atomic_bool &BuildAction::aborted() const
{
return m_aborted;
}
inline LogContext &BuildAction::log()
{
return m_log;

View File

@ -19,8 +19,8 @@ void BuildProcessSession::BuffersToWrite::clear()
outstandingBuffersToSend.clear();
}
void BuildProcessSession::DataForWebSession::streamFile(
const std::string &filePath, std::shared_ptr<WebAPI::Session> &&session, std::unique_lock<std::mutex> &&lock)
void BuildProcessSession::DataForWebSession::streamFile(const std::string &filePath, const std::shared_ptr<BuildProcessSession> &processSession,
const std::shared_ptr<WebAPI::Session> &webSession, std::unique_lock<std::mutex> &&lock)
{
error = false;
@ -61,13 +61,14 @@ void BuildProcessSession::DataForWebSession::streamFile(
return;
}
#endif
m_fileBuffer = m_session.m_bufferPool.newBuffer();
m_fileStream.async_read_some(boost::asio::buffer(*m_fileBuffer, sizeof(std::min(fileSize, m_session.m_bufferPool.bufferSize()))),
std::bind(&DataForWebSession::writeFileData, this, std::ref(filePath), std::move(session), std::placeholders::_1, std::placeholders::_2));
m_fileBuffer = processSession->m_bufferPool.newBuffer();
m_fileStream.async_read_some(boost::asio::buffer(*m_fileBuffer, sizeof(std::min(fileSize, processSession->m_bufferPool.bufferSize()))),
[this, &filePath, processSession, webSession](
auto &error, auto bytesTransferred) { writeFileData(filePath, processSession, webSession, error, bytesTransferred); });
}
void BuildProcessSession::DataForWebSession::writeFileData(
const std::string &filePath, std::shared_ptr<WebAPI::Session> session, const boost::system::error_code &readError, size_t bytesTransferred)
void BuildProcessSession::DataForWebSession::writeFileData(const std::string &filePath, const std::shared_ptr<BuildProcessSession> &processSession,
const std::shared_ptr<WebAPI::Session> &webSession, const boost::system::error_code &readError, size_t bytesTransferred)
{
// handle error
const auto eof = readError == boost::asio::error::eof;
@ -83,14 +84,14 @@ void BuildProcessSession::DataForWebSession::writeFileData(
bytesTransferred = m_bytesToSendFromFile;
}
const auto bytesLeftToRead = m_bytesToSendFromFile - bytesTransferred;
boost::beast::net::async_write(session->socket(), boost::beast::http::make_chunk(boost::asio::buffer(*m_fileBuffer, bytesTransferred)),
[this, &filePath, session, bytesLeftToRead, moreToRead = !eof && bytesLeftToRead](
boost::beast::net::async_write(webSession->socket(), boost::beast::http::make_chunk(boost::asio::buffer(*m_fileBuffer, bytesTransferred)),
[this, &filePath, processSession, webSession, bytesLeftToRead, moreToRead = !eof && bytesLeftToRead](
boost::system::error_code ecWebClient, std::size_t bytesTransferredToWebClient) {
// handle error
CPP_UTILITIES_UNUSED(bytesTransferredToWebClient)
if (ecWebClient) {
cerr << Phrases::WarningMessage << "Error sending \"" << filePath << "\" to client: " << ecWebClient.message() << Phrases::EndFlush;
std::lock_guard<std::mutex> lock(m_session.m_mutex);
std::lock_guard<std::mutex> lock(processSession->m_mutex);
clear();
error = true;
m_bytesToSendFromFile.store(0);
@ -99,16 +100,18 @@ void BuildProcessSession::DataForWebSession::writeFileData(
m_bytesToSendFromFile.store(bytesLeftToRead);
// tell the client it's over if there is nothing more to read
if (!moreToRead) {
if (m_session.m_exited.load()) {
boost::beast::net::async_write(session->socket(), boost::beast::http::make_chunk_last(),
std::bind(&WebAPI::Session::responded, session, std::placeholders::_1, std::placeholders::_2, true));
if (processSession->m_exited.load()) {
boost::beast::net::async_write(webSession->socket(), boost::beast::http::make_chunk_last(),
std::bind(&WebAPI::Session::responded, webSession, std::placeholders::_1, std::placeholders::_2, true));
}
return;
}
// continue reading if there's more data
m_fileStream.async_read_some(boost::asio::buffer(*m_fileBuffer, sizeof(std::min(bytesLeftToRead, m_session.m_bufferPool.bufferSize()))),
std::bind(
&DataForWebSession::writeFileData, this, std::ref(filePath), std::move(session), std::placeholders::_1, std::placeholders::_2));
m_fileStream.async_read_some(
boost::asio::buffer(*m_fileBuffer, sizeof(std::min(bytesLeftToRead, processSession->m_bufferPool.bufferSize()))),
[this, &filePath, processSession, webSession](auto &readError2, auto bytesTransferred2) {
writeFileData(filePath, processSession, webSession, readError2, bytesTransferred2);
});
});
}
@ -117,9 +120,9 @@ void BuildProcessSession::registerWebSession(std::shared_ptr<WebAPI::Session> &&
std::unique_lock<std::mutex> lock(m_mutex);
auto &sessionInfo = m_registeredWebSessions[webSession];
if (!sessionInfo) {
sessionInfo = std::make_unique<DataForWebSession>(*this);
sessionInfo = std::make_unique<DataForWebSession>(m_ioContext);
}
sessionInfo->streamFile(m_logFilePath, std::move(webSession), std::move(lock));
sessionInfo->streamFile(m_logFilePath, shared_from_this(), std::move(webSession), std::move(lock));
}
void BuildProcessSession::registerNewDataHandler(std::function<void(BuildProcessSession::BufferType, std::size_t)> &&handler)
@ -283,8 +286,10 @@ void BuildProcessSession::writeNextBufferToLogFile(const boost::system::error_co
m_logFileBuffers.currentlySentBufferRefs.emplace_back(boost::asio::buffer(buffer.first.get(), buffer.second));
}
}
boost::asio::async_write(m_logFileStream, m_logFileBuffers.currentlySentBufferRefs,
std::bind(&BuildProcessSession::writeNextBufferToLogFile, shared_from_this(), std::placeholders::_1, std::placeholders::_2));
if (!m_logFileBuffers.currentlySentBufferRefs.empty()) {
boost::asio::async_write(m_logFileStream, m_logFileBuffers.currentlySentBufferRefs,
std::bind(&BuildProcessSession::writeNextBufferToLogFile, shared_from_this(), std::placeholders::_1, std::placeholders::_2));
}
}
void BuildProcessSession::writeNextBufferToWebSession(
@ -356,15 +361,14 @@ void BuildProcessSession::conclude()
m_exited = true;
// detach from build action
auto buildAction = m_buildAction.lock();
if (!buildAction) {
if (!m_buildAction) {
return;
}
if (const auto outputLock = std::lock_guard<std::mutex>(buildAction->m_outputSessionMutex); buildAction->m_outputSession.get() == this) {
buildAction->m_outputSession.reset();
if (const auto outputLock = std::lock_guard<std::mutex>(m_buildAction->m_outputSessionMutex); m_buildAction->m_outputSession.get() == this) {
m_buildAction->m_outputSession.reset();
} else {
const auto processesLock = std::lock_guard<std::mutex>(buildAction->m_processesMutex);
buildAction->m_ongoingProcesses.erase(m_logFilePath);
const auto processesLock = std::lock_guard<std::mutex>(m_buildAction->m_processesMutex);
m_buildAction->m_ongoingProcesses.erase(m_logFilePath);
}
}

View File

@ -153,15 +153,15 @@ private:
void clear();
};
struct DataForWebSession : public BuffersToWrite {
explicit DataForWebSession(BuildProcessSession &session);
void streamFile(const std::string &filePath, std::shared_ptr<WebAPI::Session> &&session, std::unique_lock<std::mutex> &&lock);
explicit DataForWebSession(boost::asio::io_context &ioc);
void streamFile(const std::string &filePath, const std::shared_ptr<BuildProcessSession> &processSession,
const std::shared_ptr<WebAPI::Session> &webSession, std::unique_lock<std::mutex> &&lock);
std::size_t bytesToSendFromFile() const;
private:
void writeFileData(const std::string &filePath, std::shared_ptr<WebAPI::Session> session, const boost::system::error_code &error,
std::size_t bytesTransferred);
void writeFileData(const std::string &filePath, const std::shared_ptr<BuildProcessSession> &processSession,
const std::shared_ptr<WebAPI::Session> &webSession, const boost::system::error_code &error, std::size_t bytesTransferred);
BuildProcessSession &m_session;
std::atomic<std::size_t> m_bytesToSendFromFile = 0;
#ifndef BOOST_ASIO_HAS_FILE
boost::beast::file m_file;
@ -180,7 +180,7 @@ private:
void close();
void conclude();
std::weak_ptr<BuildAction> m_buildAction;
std::shared_ptr<BuildAction> m_buildAction;
boost::process::async_pipe m_pipe;
BufferPoolType m_bufferPool;
BufferType m_buffer;
@ -198,9 +198,8 @@ private:
std::atomic_bool m_exited = false;
};
inline BuildProcessSession::DataForWebSession::DataForWebSession(BuildProcessSession &session)
: m_session(session)
, m_fileStream(session.m_ioContext)
inline BuildProcessSession::DataForWebSession::DataForWebSession(boost::asio::io_context &ioc)
: m_fileStream(ioc)
{
}
@ -249,8 +248,9 @@ template <typename... ChildArgs> void BuildProcessSession::launch(ChildArgs &&..
m_ioContext, group, std::forward<ChildArgs>(childArgs)..., (boost::process::std_out & boost::process::std_err) > m_pipe,
boost::process::extend::on_success =
[session = shared_from_this()](auto &executor) {
if (const auto buildAction = session->m_buildAction.lock()) {
buildAction->appendOutput(CppUtilities::EscapeCodes::Phrases::InfoMessage, "Launched \"", session->m_displayName, "\", PID: ",
if (session->m_buildAction) {
session->m_buildAction->appendOutput(CppUtilities::EscapeCodes::Phrases::InfoMessage, "Launched \"", session->m_displayName,
"\", PID: ",
executor
#ifdef PLATFORM_WINDOWS
.proc_info.dwProcessId

View File

@ -241,10 +241,11 @@ void LibRepoMgr::ReloadLibraryDependencies::downloadPackagesFromMirror()
}
m_buildAction->appendOutput(Phrases::SuccessMessage, "Downloading ", packagesWhichNeedCaching, " binary packages from mirror ...\n");
WebClient::cachePackages(m_buildAction->log(),
std::make_shared<WebClient::PackageCachingSession>(m_cachingData, m_setup.building.ioContext, m_setup.webServer.sslContext,
std::bind(&ReloadLibraryDependencies::loadPackageInfoFromContents, this)),
m_packageDownloadSizeLimit ? std::make_optional(m_packageDownloadSizeLimit) : std::nullopt);
auto session = std::make_shared<WebClient::PackageCachingSession>(m_cachingData, m_setup.building.ioContext, m_setup.webServer.sslContext,
std::bind(&ReloadLibraryDependencies::loadPackageInfoFromContents, this));
session->aborted = &m_buildAction->aborted();
WebClient::cachePackages(
m_buildAction->log(), std::move(session), m_packageDownloadSizeLimit ? std::make_optional(m_packageDownloadSizeLimit) : std::nullopt);
}
void ReloadLibraryDependencies::loadPackageInfoFromContents()

View File

@ -228,7 +228,8 @@ PackageCachingDataForPackage *PackageCachingSession::getCurrentDataAndSelectNext
void cachePackages(LogContext &log, std::shared_ptr<PackageCachingSession> &&packageCachingSession, std::optional<std::uint64_t> bodyLimit,
std::size_t maxParallelDownloads)
{
for (std::size_t startedDownloads = 0; startedDownloads < maxParallelDownloads; ++startedDownloads) {
for (std::size_t startedDownloads = 0;
startedDownloads < maxParallelDownloads && (!packageCachingSession->aborted || !*packageCachingSession->aborted); ++startedDownloads) {
auto *const cachingData = packageCachingSession->getCurrentDataAndSelectNext();
if (!cachingData) {
return;
@ -243,7 +244,7 @@ void cachePackages(LogContext &log, std::shared_ptr<PackageCachingSession> &&pac
cachingData->error = tupleToString(msg);
log(Phrases::ErrorMessage, msg, '\n');
}
const auto &response = get<FileResponse>(session.response);
const auto &response = std::get<FileResponse>(session.response);
const auto &message = response.get();
if (message.result() != boost::beast::http::status::ok) {
const auto msg = std::make_tuple("Error downloading \"", cachingData->url, "\" to \"", cachingData->destinationFilePath,

View File

@ -55,6 +55,8 @@ struct PackageCachingSession : public MultiSession<void> {
explicit PackageCachingSession(
PackageCachingDataForSession &data, boost::asio::io_context &ioContext, boost::asio::ssl::context &sslContext, HandlerType &&handler);
const std::atomic_bool *aborted = nullptr;
private:
void selectNextPackage();
PackageCachingDataForPackage *getCurrentDataAndSelectNext();