From d7737cc84f8e442fac5d246b1d4ec063312ea259 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Wed, 29 Jul 2009 15:04:02 +0000 Subject: [PATCH] Changed the somewhat bold BLocker usage to a more straight forward semaphore based mechanism to pause and eventually cancel the WorkerThread. This also works now when the CopyEngine is still collecting file informartion (ticket #4153). git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@31912 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/apps/installer/CopyEngine.cpp | 73 ++++++++++++++++++++------ src/apps/installer/CopyEngine.h | 13 +++-- src/apps/installer/InstallerWindow.cpp | 55 +++++++++---------- src/apps/installer/InstallerWindow.h | 5 +- src/apps/installer/WorkerThread.cpp | 11 ++-- src/apps/installer/WorkerThread.h | 7 ++- 6 files changed, 104 insertions(+), 60 deletions(-) diff --git a/src/apps/installer/CopyEngine.cpp b/src/apps/installer/CopyEngine.cpp index 9537b53551..75940743e5 100644 --- a/src/apps/installer/CopyEngine.cpp +++ b/src/apps/installer/CopyEngine.cpp @@ -26,6 +26,39 @@ using std::nothrow; +// SemaphoreLocking +class SemaphoreLocking { +public: + inline bool Lock(sem_id* lockable) + { + return acquire_sem(*lockable) == B_OK; + } + + inline void Unlock(sem_id* lockable) + { + release_sem(*lockable); + } +}; + +// SemaphoreLocker +class SemaphoreLocker : public AutoLocker { +public: + inline SemaphoreLocker(sem_id semaphore, bool alreadyLocked = false, + bool lockIfNotLocked = true) + : + AutoLocker(), + fSem(semaphore) + { + SetTo(&fSem, alreadyLocked, lockIfNotLocked); + } + +private: + sem_id fSem; +}; + + + + CopyEngine::CopyEngine(const BMessenger& messenger, BMessage* message) : fBufferQueue(), @@ -102,16 +135,16 @@ CopyEngine::ResetTargets() status_t -CopyEngine::CollectTargets(const char* source) +CopyEngine::CollectTargets(const char* source, sem_id cancelSemaphore) { int32 level = 0; - return _CollectCopyInfo(source, level); + return _CollectCopyInfo(source, level, cancelSemaphore); } status_t CopyEngine::CopyFolder(const char* source, const char* destination, - BLocker* locker) + sem_id cancelSemaphore) { printf("%lld bytes to read in %lld files\n", fBytesToCopy, fItemsToCopy); @@ -122,16 +155,16 @@ CopyEngine::CopyFolder(const char* source, const char* destination, } int32 level = 0; - return _CopyFolder(source, destination, level, locker); + return _CopyFolder(source, destination, level, cancelSemaphore); } status_t CopyEngine::CopyFile(const BEntry& _source, const BEntry& _destination, - BLocker* locker) + sem_id cancelSemaphore) { - AutoLocker lock(locker); - if (locker != NULL && !lock.IsLocked()) { + SemaphoreLocker lock(cancelSemaphore); + if (cancelSemaphore >= 0 && !lock.IsLocked()) { // We are supposed to quit printf("CopyFile - cancled\n"); return B_CANCELED; @@ -210,7 +243,8 @@ printf("CopyFile - cancled\n"); status_t -CopyEngine::_CollectCopyInfo(const char* _source, int32& level) +CopyEngine::_CollectCopyInfo(const char* _source, int32& level, + sem_id cancelSemaphore) { level++; @@ -221,6 +255,12 @@ CopyEngine::_CollectCopyInfo(const char* _source, int32& level) BEntry entry; while (source.GetNextEntry(&entry) == B_OK) { + SemaphoreLocker lock(cancelSemaphore); + if (cancelSemaphore >= 0 && !lock.IsLocked()) { + // We are supposed to quit + return B_CANCELED; + } + struct stat statInfo; entry.GetStat(&statInfo); @@ -239,7 +279,10 @@ CopyEngine::_CollectCopyInfo(const char* _source, int32& level) if (ret < B_OK) return ret; - ret = _CollectCopyInfo(srcFolder.Path(), level); + if (cancelSemaphore >= 0) + lock.Unlock(); + + ret = _CollectCopyInfo(srcFolder.Path(), level, cancelSemaphore); if (ret < B_OK) return ret; } else if (S_ISLNK(statInfo.st_mode)) { @@ -259,7 +302,7 @@ CopyEngine::_CollectCopyInfo(const char* _source, int32& level) status_t CopyEngine::_CopyFolder(const char* _source, const char* _destination, - int32& level, BLocker* locker) + int32& level, sem_id cancelSemaphore) { level++; fCurrentTargetFolder = _destination; @@ -283,8 +326,8 @@ CopyEngine::_CopyFolder(const char* _source, const char* _destination, BEntry entry; while (source.GetNextEntry(&entry) == B_OK) { - AutoLocker lock(locker); - if (locker != NULL && !lock.IsLocked()) { + SemaphoreLocker lock(cancelSemaphore); + if (cancelSemaphore >= 0 && !lock.IsLocked()) { // We are supposed to quit return B_CANCELED; } @@ -341,15 +384,15 @@ CopyEngine::_CopyFolder(const char* _source, const char* _destination, if (ret < B_OK) return ret; - if (locker != NULL) + if (cancelSemaphore >= 0) lock.Unlock(); ret = _CopyFolder(srcFolder.Path(), dstFolder.Path(), level, - locker); + cancelSemaphore); if (ret < B_OK) return ret; - if (locker != NULL && !lock.Lock()) { + if (cancelSemaphore >= 0 && !lock.Lock()) { // We are supposed to quit return B_CANCELED; } diff --git a/src/apps/installer/CopyEngine.h b/src/apps/installer/CopyEngine.h index 1ad12b44ce..3f027e23f3 100644 --- a/src/apps/installer/CopyEngine.h +++ b/src/apps/installer/CopyEngine.h @@ -15,7 +15,6 @@ #include "BlockingQueue.h" class BFile; -class BLocker; class BMessage; class BMessenger; @@ -26,23 +25,23 @@ public: virtual ~CopyEngine(); void ResetTargets(); - status_t CollectTargets(const char* source); + status_t CollectTargets(const char* source, + sem_id cancelSemaphore = -1); status_t CopyFolder(const char* source, const char* destination, - BLocker* locker = NULL); + sem_id cancelSemaphore = -1); status_t CopyFile(const BEntry& entry, const BEntry& destination, - BLocker* locker = NULL); + sem_id cancelSemaphore = -1); private: status_t _CollectCopyInfo(const char* source, - int32& level); + int32& level, sem_id cancelSemaphore); status_t _CopyFolder(const char* source, const char* destination, - int32& level, - BLocker* locker = NULL); + int32& level, sem_id cancelSemaphore); bool _ShouldCopyEntry(const char* name, const struct stat& statInfo, diff --git a/src/apps/installer/InstallerWindow.cpp b/src/apps/installer/InstallerWindow.cpp index fbc70bef98..2d5e5d4597 100644 --- a/src/apps/installer/InstallerWindow.cpp +++ b/src/apps/installer/InstallerWindow.cpp @@ -149,7 +149,7 @@ InstallerWindow::InstallerWindow() fDriveSetupLaunched(false), fInstallStatus(kReadyForInstall), fWorkerThread(new WorkerThread(this)), - fCopyEngineLock(NULL) + fCopyEngineCancelSemaphore(-1) { LogoView* logoView = new LogoView(); @@ -271,6 +271,7 @@ InstallerWindow::InstallerWindow() InstallerWindow::~InstallerWindow() { + _SetCopyEngineCancelSemaphore(-1); be_roster->StopWatching(this); } @@ -299,8 +300,7 @@ InstallerWindow::MessageReceived(BMessage *msg) switch (msg->what) { case MSG_RESET: { - delete fCopyEngineLock; - fCopyEngineLock = NULL; + _SetCopyEngineCancelSemaphore(-1); status_t error; if (msg->FindInt32("error", &error) == B_OK) { @@ -326,12 +326,13 @@ InstallerWindow::MessageReceived(BMessage *msg) switch (fInstallStatus) { case kReadyForInstall: { - delete fCopyEngineLock; - fCopyEngineLock = new BLocker("copy engine lock"); + _SetCopyEngineCancelSemaphore(create_sem(1, + "copy engine cancel")); + BList* list = new BList(); int32 size = 0; fPackagesView->GetPackagesToInstall(list, &size); - fWorkerThread->SetLock(fCopyEngineLock); + fWorkerThread->SetLock(fCopyEngineCancelSemaphore); fWorkerThread->SetPackagesList(list); fWorkerThread->SetSpaceRequired(size); fInstallStatus = kInstalling; @@ -415,8 +416,8 @@ InstallerWindow::MessageReceived(BMessage *msg) } case MSG_INSTALL_FINISHED: { - delete fCopyEngineLock; - fCopyEngineLock = NULL; + + _SetCopyEngineCancelSemaphore(-1); fBeginButton->SetLabel("Quit"); @@ -688,14 +689,27 @@ InstallerWindow::_SetStatusMessage(const char *text) } +void +InstallerWindow::_SetCopyEngineCancelSemaphore(sem_id id, bool alreadyLocked) +{ + if (fCopyEngineCancelSemaphore >= 0) { + if (!alreadyLocked) + acquire_sem(fCopyEngineCancelSemaphore); + delete_sem(fCopyEngineCancelSemaphore); + } + fCopyEngineCancelSemaphore = id; +} + + void InstallerWindow::_QuitCopyEngine(bool askUser) { - if (fCopyEngineLock == NULL) + if (fCopyEngineCancelSemaphore < 0) return; - // first of all block the copy engine - fCopyEngineLock->Lock(); + // First of all block the copy engine, so that it doesn't continue + // while the alert is showing, which would be irritating. + acquire_sem(fCopyEngineCancelSemaphore); bool quit = true; if (askUser) { @@ -706,23 +720,10 @@ InstallerWindow::_QuitCopyEngine(bool askUser) } if (quit) { - int32 tries = 0; - // wait until the engine blocks - while (fCopyEngineLock->CountLockRequests() < 2) { - // TODO: There is a race here, the copy engine - // may have finished before we locked the engine - // lock. That's why we limit the number of tries - // here. - tries++; - if (tries > 100) - break; - snooze(3000); - } - // make it quit by having it's lock fail - delete fCopyEngineLock; - fCopyEngineLock = NULL; + // Make it quit by having it's lock fail... + _SetCopyEngineCancelSemaphore(-1, true); } else - fCopyEngineLock->Unlock(); + release_sem(fCopyEngineCancelSemaphore); } diff --git a/src/apps/installer/InstallerWindow.h b/src/apps/installer/InstallerWindow.h index 04288e591e..4c650619a8 100644 --- a/src/apps/installer/InstallerWindow.h +++ b/src/apps/installer/InstallerWindow.h @@ -16,7 +16,6 @@ using namespace BPrivate; class BButton; class BLayoutItem; -class BLocker; class BMenu; class BMenuField; class BStatusBar; @@ -61,6 +60,8 @@ private: void _PublishPackages(); void _SetStatusMessage(const char* text); + void _SetCopyEngineCancelSemaphore(sem_id id, + bool alreadyLocked = false); void _QuitCopyEngine(bool askUser); static int _ComparePackages(const void* firstArg, @@ -95,7 +96,7 @@ private: WorkerThread* fWorkerThread; BString fLastStatus; - BLocker* fCopyEngineLock; + sem_id fCopyEngineCancelSemaphore; }; #endif // INSTALLER_WINDOW_H diff --git a/src/apps/installer/WorkerThread.cpp b/src/apps/installer/WorkerThread.cpp index f23feae8bf..04f48c00d1 100644 --- a/src/apps/installer/WorkerThread.cpp +++ b/src/apps/installer/WorkerThread.cpp @@ -77,7 +77,8 @@ WorkerThread::WorkerThread(InstallerWindow *window) : BLooper("copy_engine"), fWindow(window), fPackages(NULL), - fSpaceRequired(0) + fSpaceRequired(0), + fCancelSemaphore(-1) { Run(); } @@ -398,7 +399,7 @@ WorkerThread::_PerformInstall(BMenu* srcMenu, BMenu* targetMenu) // let the engine collect information for the progress bar later on engine.ResetTargets(); - err = engine.CollectTargets(srcDirectory.Path()); + err = engine.CollectTargets(srcDirectory.Path(), fCancelSemaphore); if (err != B_OK) goto error; @@ -409,7 +410,7 @@ WorkerThread::_PerformInstall(BMenu* srcMenu, BMenu* targetMenu) for (int32 i = 0; i < count; i++) { Package *p = static_cast(fPackages->ItemAt(i)); BPath packageDir(pkgRootDir.Path(), p->Folder()); - err = engine.CollectTargets(packageDir.Path()); + err = engine.CollectTargets(packageDir.Path(), fCancelSemaphore); if (err != B_OK) goto error; } @@ -417,7 +418,7 @@ WorkerThread::_PerformInstall(BMenu* srcMenu, BMenu* targetMenu) // copy source volume err = engine.CopyFolder(srcDirectory.Path(), targetDirectory.Path(), - fCancelLock); + fCancelSemaphore); if (err != B_OK) goto error; @@ -429,7 +430,7 @@ WorkerThread::_PerformInstall(BMenu* srcMenu, BMenu* targetMenu) Package *p = static_cast(fPackages->ItemAt(i)); BPath packageDir(pkgRootDir.Path(), p->Folder()); err = engine.CopyFolder(packageDir.Path(), targetDirectory.Path(), - fCancelLock); + fCancelSemaphore); if (err != B_OK) goto error; } diff --git a/src/apps/installer/WorkerThread.h b/src/apps/installer/WorkerThread.h index b5b2b7547d..25f58ed69e 100644 --- a/src/apps/installer/WorkerThread.h +++ b/src/apps/installer/WorkerThread.h @@ -14,7 +14,6 @@ #include class BList; -class BLocker; class BMenu; class InstallerWindow; @@ -32,8 +31,8 @@ public: { fSpaceRequired = bytes; }; bool Cancel(); - void SetLock(BLocker* lock) - { fCancelLock = lock; } + void SetLock(sem_id cancelSemaphore) + { fCancelSemaphore = cancelSemaphore; } void StartInstall(); void WriteBootSector(BMenu* dstMenu); @@ -50,7 +49,7 @@ private: BDiskDeviceRoster fDDRoster; BList* fPackages; off_t fSpaceRequired; - BLocker* fCancelLock; + sem_id fCancelSemaphore; }; #endif // WORKER_THREAD_H