Allow to abort build action immediately while waiting for chroot locks

* Avoid aborted build actions from being stuck in running state while
  waiting until the chroot locks can be acquired; allow aborting action
  immediately instead
* Use async locks when invoking makechrootpkg and split affected function
  accordingly
This commit is contained in:
Martchus 2022-12-30 21:12:49 +01:00
parent b8f211e1d1
commit 4f413e86ba
4 changed files with 95 additions and 5 deletions

View File

@ -345,9 +345,56 @@ void BuildAction::assignStartAfter(const std::vector<std::shared_ptr<BuildAction
void BuildAction::abort()
{
m_aborted.store(true);
if (m_setup && m_stopHandler) {
if (!m_setup) {
return;
}
if (m_stopHandler) {
boost::asio::post(m_setup->building.ioContext.get_executor(), m_stopHandler);
}
if (m_waitingOnAsyncLock) {
const auto buildActionLock = m_setup->building.lockToWrite();
if (isExecuting()) {
conclude(BuildActionResult::Aborted);
}
}
}
void LibRepoMgr::BuildAction::acquireToWrite(std::string &&lockName, std::move_only_function<void(UniqueLoggingLock &&)> &&callback)
{
// flag this action as "waiting for async lock" so the abort() function is allowed to conclude the action right away (and thus the
// build action is not stuck in "running" until the lock is acquired)
m_waitingOnAsyncLock.store(true);
// handle abortion if aborted (instead of acquiring the lock)
if (m_aborted) {
auto buildActionLock = m_setup->building.lockToWrite();
if (isExecuting()) {
conclude(BuildActionResult::Aborted);
}
return;
}
// acquire the lock asynchronously
m_setup->locks.acquireToWrite(
log(), std::move(lockName), [t = shared_from_this(), callback = std::move(callback)](UniqueLoggingLock &&lock) mutable {
// stop the abort() function from immediately concluding the build action again
t->m_waitingOnAsyncLock.store(false);
// execute the callback in another building thread to avoid interferances with the thread that invoked this callback (when releasing its own lock)
boost::asio::post(t->m_setup->building.ioContext.get_executor(), [t = t, callback = std::move(callback), lock = std::move(lock)] mutable {
// conclude the action as aborted if it has been aborted meanwhile
auto buildActionLock = t->m_setup->building.lockToWrite();
if (t->m_aborted && t->isExecuting()) {
t->conclude(BuildActionResult::Aborted);
}
// execute the callback only if the action hasn't been aborted
if (!t->m_aborted) {
buildActionLock.unlock();
callback(std::move(lock));
}
});
});
}
template <typename InternalBuildActionType> void BuildAction::post()

View File

@ -215,6 +215,7 @@ public:
LibPkg::StorageID start(ServiceSetup &setup, std::unique_ptr<Io::PasswordFile> &&secrets);
void assignStartAfter(const std::vector<std::shared_ptr<BuildAction>> &startsAfterBuildActions);
void abort();
void acquireToWrite(std::string &&lockName, std::move_only_function<void(UniqueLoggingLock &&lock)> &&callback);
void appendOutput(std::string_view output);
void appendOutput(std::string &&output);
template <typename... Args> void appendOutput(Args &&...args);
@ -254,6 +255,7 @@ private:
LogContext m_log;
ServiceSetup *m_setup = nullptr;
std::atomic_bool m_aborted = false;
std::atomic_bool m_waitingOnAsyncLock = false;
std::function<void(void)> m_stopHandler;
std::function<void(void)> m_concludeHandler;
std::mutex m_processesMutex;

View File

@ -596,6 +596,14 @@ private:
const std::string &packageName, PackageBuildProgress &packageProgress, const std::string &buildDirectory);
void invokeMakechrootpkg(const BatchProcessingSession::SharedPointerType &makepkgchrootSession, const std::string &packageName,
bool hasFailuresInPreviousBatches, std::move_only_function<void(InvocationResult)> &&cb);
void invokeMakechrootpkgStep2(const BatchProcessingSession::SharedPointerType &makepkgchrootSession, const std::string &packageName,
UniqueLoggingLock &&chrootLock, const std::string &chrootDir, const std::string &buildRoot,
const std::vector<std::string> &makechrootpkgFlags, const std::vector<std::string> &makepkgFlags, const std::vector<std::string> &sudoArgs,
std::move_only_function<void(InvocationResult)> &&cb);
void invokeMakechrootpkgStep3(std::shared_ptr<BuildProcessSession> &processSession, const std::string &packageName,
UniqueLoggingLock &&chrootLock, UniqueLoggingLock &&chrootUserLock, const std::string &chrootDir,
const std::vector<std::string> &makechrootpkgFlags, const std::vector<std::string> &makepkgFlags, const std::vector<std::string> &sudoArgs,
std::move_only_function<void(InvocationResult)> &&cb);
void invokeMakecontainerpkg(const BatchProcessingSession::SharedPointerType &makepkgchrootSession, const std::string &packageName,
PackageBuildProgress &packageProgress, const std::vector<std::string> &makepkgFlags, std::shared_lock<std::shared_mutex> &&lock,
std::move_only_function<void(InvocationResult)> &&cb);

View File

@ -1063,9 +1063,21 @@ void ConductBuild::invokeMakechrootpkg(const BatchProcessingSession::SharedPoint
}
// lock the chroot directory to prevent other build tasks from using it
auto lockName = std::string(buildRoot);
m_buildAction->log()(Phrases::InfoMessage, "Building ", packageName, '\n');
auto chrootLock = m_setup.locks.acquireToWrite(m_buildAction->log(), std::string(buildRoot));
m_buildAction->acquireToWrite(std::move(lockName),
[this, makepkgchrootSession, &packageName, chrootDir = std::move(chrootDir), buildRoot = std::move(buildRoot),
makechrootpkgFlags = std::move(makechrootpkgFlags), makepkgFlags = std::move(makepkgFlags), sudoArgs = std::move(sudoArgs),
cb = std::move(cb)](UniqueLoggingLock &&chrootLock) mutable {
invokeMakechrootpkgStep2(makepkgchrootSession, packageName, std::move(chrootLock), chrootDir, buildRoot, makechrootpkgFlags, makepkgFlags,
sudoArgs, std::move(cb));
});
}
void ConductBuild::invokeMakechrootpkgStep2(const BatchProcessingSession::SharedPointerType &makepkgchrootSession, const std::string &packageName,
UniqueLoggingLock &&chrootLock, const std::string &chrootDir, const std::string &buildRoot, const std::vector<std::string> &makechrootpkgFlags,
const std::vector<std::string> &makepkgFlags, const std::vector<std::string> &sudoArgs, std::move_only_function<void(InvocationResult)> &&cb)
{
// copy config files into chroot directory
try {
std::filesystem::copy_file(makepkgchrootSession->isStagingEnabled() ? m_pacmanStagingConfigPath : m_pacmanConfigPath,
@ -1073,13 +1085,15 @@ void ConductBuild::invokeMakechrootpkg(const BatchProcessingSession::SharedPoint
std::filesystem::copy_file(m_makepkgConfigPath, buildRoot + "/etc/makepkg.conf", std::filesystem::copy_options::overwrite_existing);
} catch (const std::filesystem::filesystem_error &e) {
chrootLock.lock().unlock();
auto writeLock = lockToWrite(lock);
packageProgress.error = "Unable to configure chroot \"" % buildRoot % "\": " + e.what();
auto writeLock = lockToWrite();
m_buildProgress.progressByPackage[packageName].error = "Unable to configure chroot \"" % buildRoot % "\": " + e.what();
writeLock.unlock();
return cb(InvocationResult::Error);
}
// prepare process session (after configuring chroot so we don't get stuck if configuring chroot fails)
auto lock = lockToRead();
auto &packageProgress = m_buildProgress.progressByPackage[packageName];
auto processSession = m_buildAction->makeBuildProcess(packageName + " build", packageProgress.buildDirectory + "/build.log",
std::bind(&ConductBuild::handleMakechrootpkgErrorsAndAddPackageToRepo, this, makepkgchrootSession, std::ref(packageName),
std::ref(packageProgress), std::placeholders::_1, std::placeholders::_2));
@ -1088,6 +1102,7 @@ void ConductBuild::invokeMakechrootpkg(const BatchProcessingSession::SharedPoint
&ConductBuild::assignNewVersion, this, std::ref(packageName), std::ref(packageProgress), std::placeholders::_1, std::placeholders::_2)));
processSession->registerNewDataHandler(BufferSearch("Synchronizing chroot copy", "\n", std::string_view(),
[processSession = processSession.get()](BufferSearch &, std::string &&) { processSession->locks().pop_back(); }));
lock.unlock();
// invoke makechrootpkg to build package
m_buildAction->log()(Phrases::InfoMessage, "Invoking makechrootpkg for ", packageName, " via ", m_makeChrootPkgPath.string(), '\n',
@ -1095,8 +1110,26 @@ void ConductBuild::invokeMakechrootpkg(const BatchProcessingSession::SharedPoint
ps(Phrases::SubMessage), "chroot user: ", packageProgress.chrootUser, '\n');
auto &locks = processSession->locks();
locks.reserve(2);
locks.emplace_back(m_setup.locks.acquireToWrite(m_buildAction->log(), chrootDir % '/' + packageProgress.chrootUser));
m_buildAction->acquireToWrite(chrootDir % '/' + packageProgress.chrootUser,
[this, processSession, &packageName, chrootLock = std::move(chrootLock), chrootDir = std::move(chrootDir),
makechrootpkgFlags = std::move(makechrootpkgFlags), makepkgFlags = std::move(makepkgFlags), sudoArgs = std::move(sudoArgs),
cb = std::move(cb)](UniqueLoggingLock &&chrootUserLock) mutable {
invokeMakechrootpkgStep3(processSession, packageName, std::move(chrootLock), std::move(chrootUserLock), chrootDir, makechrootpkgFlags,
makepkgFlags, sudoArgs, std::move(cb));
});
}
void ConductBuild::invokeMakechrootpkgStep3(std::shared_ptr<BuildProcessSession> &processSession, const std::string &packageName,
UniqueLoggingLock &&chrootLock, UniqueLoggingLock &&chrootUserLock, const std::string &chrootDir,
const std::vector<std::string> &makechrootpkgFlags, const std::vector<std::string> &makepkgFlags, const std::vector<std::string> &sudoArgs,
std::move_only_function<void(InvocationResult)> &&cb)
{
// invoke makechrootpkg to build package
auto &locks = processSession->locks();
locks.emplace_back(std::move(chrootUserLock));
locks.emplace_back(std::move(chrootLock));
auto lock = lockToRead();
auto &packageProgress = m_buildProgress.progressByPackage[packageName];
processSession->launch(boost::process::start_dir(packageProgress.buildDirectory), m_makeChrootPkgPath, sudoArgs, makechrootpkgFlags, "-C",
m_globalPackageCacheDir, "-r", chrootDir, "-l", packageProgress.chrootUser, packageProgress.makechrootpkgFlags, "--", makepkgFlags,
packageProgress.makepkgFlags, boost::process::std_in < boost::asio::buffer(m_sudoPassword));