SVN commit 1213836 by dfaure: Fix r1210779 so that it uses a function rather than a file-static int (== global initializer), and explain in kdebug.h why people shouldn't use a global initializer (in practice, it broke kconfigafterkglobaltest[12] and kdebug_unittest) CCMAIL: mpyne@kde.org M +3 -0 io/kdebug.h M +5 -1 tests/kconfigafterkglobaltest1.cpp M +39 -35 util/kshareddatacache.cpp M +2 -2 util/kshareddatacache_p.h --- trunk/KDE/kdelibs/kdecore/io/kdebug.h #1213835:1213836 @@ -288,6 +288,9 @@ * * static int debugArea() { static int s_area = KDebug::registerArea("areaName"); return s_area; } * + * Please do not use a file-static int, it would (indirectly) create KGlobal too early, + * create KConfig instances too early (breaking unittests which set KDEHOME), etc. + * By using a function as shown above, you make it all happen on-demand, rather than upfront. * * If all uses of the debug area are restricted to a single .cpp file, do the same * but outside any class, and then use a more specific name for the function. --- trunk/KDE/kdelibs/kdecore/tests/kconfigafterkglobaltest1.cpp #1213835:1213836 @@ -54,7 +54,11 @@ Tester::~Tester() { - Q_ASSERT(!KGlobal::hasMainComponent()); // the KGlobal K_GLOBAL_STATIC should already be deleted + // the KGlobal K_GLOBAL_STATIC should already be deleted + // If this fails, check that a new global static in kdecore didn't create KGlobal indirectly; + // e.g. this can happen if someone writes 'static int s_area = KDebug::registerArea("foo");' in + // a .cpp file, rather than using a function. + Q_ASSERT(!KGlobal::hasMainComponent()); KConfigGroup group = m_config->group("test"); group.writeEntry("test", 1); delete m_config; // this calls KConfig::sync() which needs KGlobal::locale() --- trunk/KDE/kdelibs/kdecore/util/kshareddatacache.cpp #1213835:1213836 @@ -41,7 +41,11 @@ #include #include -// Debug area "s_area" is defined in the kshareddatacache_p.h +int ksdcArea() +{ + static int s_ksdcArea = KDebug::registerArea("KSharedDataCache", false); + return s_ksdcArea; +} //----------------------------------------------------------------------------- // MurmurHashAligned, by Austin Appleby @@ -378,19 +382,19 @@ bool performInitialSetup(uint _cacheSize, uint _pageSize) { if (_cacheSize < MINIMUM_CACHE_SIZE) { - kError(s_area) << "Internal error: Attempted to create a cache sized < " + kError(ksdcArea()) << "Internal error: Attempted to create a cache sized < " << MINIMUM_CACHE_SIZE; return false; } if (_pageSize == 0) { - kError(s_area) << "Internal error: Attempted to create a cache with 0-sized pages."; + kError(ksdcArea()) << "Internal error: Attempted to create a cache with 0-sized pages."; return false; } shmLock.type = findBestSharedLock(); if (static_cast(shmLock.type) == 0) { - kError(s_area) << "Unable to find an appropriate lock to guard the shared cache. " + kError(ksdcArea()) << "Unable to find an appropriate lock to guard the shared cache. " << "This *should* be essentially impossible. :("; return false; } @@ -399,12 +403,12 @@ QSharedPointer tempLock(createLockFromId(shmLock.type, shmLock)); if (!tempLock->initialize(isProcessShared)) { - kError(s_area) << "Unable to initialize the lock for the cache!"; + kError(ksdcArea()) << "Unable to initialize the lock for the cache!"; return false; } if (!isProcessShared) { - kWarning(s_area) << "Cache initialized, but does not support being" + kWarning(ksdcArea()) << "Cache initialized, but does not support being" << "shared across processes."; } @@ -603,7 +607,7 @@ return; // That was easy } - kDebug(s_area) << "Defragmenting the shared cache"; + kDebug(ksdcArea()) << "Defragmenting the shared cache"; // Just do a linear scan, and anytime there is free space, swap it // with the pages to its right. In order to meet the precondition @@ -724,13 +728,13 @@ uint removeUsedPages(uint numberNeeded) { if (numberNeeded == 0) { - kError(s_area) << "Internal error: Asked to remove exactly 0 pages for some reason."; + kError(ksdcArea()) << "Internal error: Asked to remove exactly 0 pages for some reason."; return pageTableSize(); } if (numberNeeded > pageTableSize()) { - kError(s_area) << "Internal error: Requested more space than exists in the cache."; - kError(s_area) << numberNeeded << "requested, " << pageTableSize() << "is the total possible."; + kError(ksdcArea()) << "Internal error: Requested more space than exists in the cache."; + kError(ksdcArea()) << numberNeeded << "requested, " << pageTableSize() << "is the total possible."; return pageTableSize(); } @@ -744,7 +748,7 @@ freedPagesRequired = numberNeeded - cacheAvail; } - kDebug(s_area) << "Removing old entries to free up" << numberNeeded << "pages," + kDebug(ksdcArea()) << "Removing old entries to free up" << numberNeeded << "pages," << cacheAvail << "are already theoretically available."; if (cacheAvail > 3 * numberNeeded) { @@ -755,7 +759,7 @@ return result; } else { - kError(s_area) << "Just defragmented a locked cache, but still there" + kError(ksdcArea()) << "Just defragmented a locked cache, but still there" << "isn't enough room for the current request."; } } @@ -766,7 +770,7 @@ QSharedPointer tablePtr(new IndexTableEntry[indexTableSize()], deleteTable); if (!tablePtr) { - kError(s_area) << "Unable to allocate temporary memory for sorting the cache!"; + kError(ksdcArea()) << "Unable to allocate temporary memory for sorting the cache!"; clearInternalTables(); return pageTableSize(); } @@ -824,12 +828,12 @@ // pagesRemoved < numberNeeded or in other words we can't fulfill // the request even if we defragment. This is really a logic error. if (curIndex < 0) { - kError(s_area) << "Unable to remove enough used pages to allocate" + kError(ksdcArea()) << "Unable to remove enough used pages to allocate" << numberNeeded << "pages. In theory the cache is empty, weird."; return pageTableSize(); } - kDebug(s_area) << "Removing entry of" << indexTable()[curIndex].totalItemSize + kDebug(ksdcArea()) << "Removing entry of" << indexTable()[curIndex].totalItemSize << "size"; removeEntry(curIndex); } @@ -949,7 +953,7 @@ void *mapAddress = MAP_FAILED; if (size < cacheSize) { - kError(s_area) << "Asked for a cache size less than requested size somehow -- Logic Error :("; + kError(ksdcArea()) << "Asked for a cache size less than requested size somehow -- Logic Error :("; return; } @@ -976,7 +980,7 @@ if (mapped->version != SharedMemory::PIXMAP_CACHE_VERSION && mapped->version > 0) { - kWarning(s_area) << "Deleting wrong version of cache" << cacheName; + kWarning(ksdcArea()) << "Deleting wrong version of cache" << cacheName; // CAUTION: Potentially recursive since the recovery // involves calling this function again. @@ -1013,7 +1017,7 @@ // shared memory. If we don't get shared memory the disk info is ignored, // if we do get shared memory we never look at disk again. if (mapAddress == MAP_FAILED) { - kWarning(s_area) << "Failed to establish shared memory mapping, will fallback" + kWarning(ksdcArea()) << "Failed to establish shared memory mapping, will fallback" << "to private memory -- memory usage will increase"; mapAddress = ::mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); @@ -1022,7 +1026,7 @@ // Well now we're really hosed. We can still work, but we can't even cache // data. if (mapAddress == MAP_FAILED) { - kError(s_area) << "Unable to allocate shared memory segment for shared data cache" + kError(ksdcArea()) << "Unable to allocate shared memory segment for shared data cache" << cacheName << "of size" << cacheSize; return; } @@ -1044,7 +1048,7 @@ while (shm->ready != 2) { if (usecSleepTime >= (1 << 21)) { // Didn't acquire within ~8 seconds? Assume an issue exists - kError(s_area) << "Unable to acquire shared lock, is the cache corrupt?"; + kError(ksdcArea()) << "Unable to acquire shared lock, is the cache corrupt?"; ::munmap(shm, size); file.remove(); // Unlink the cache in case it's corrupt. @@ -1054,7 +1058,7 @@ if (shm->ready.testAndSetAcquire(0, 1)) { if (!shm->performInitialSetup(cacheSize, pageSize)) { - kError(s_area) << "Unable to perform initial setup, this system probably " + kError(ksdcArea()) << "Unable to perform initial setup, this system probably " "does not really support process-shared pthreads or " "semaphores, even though it claims otherwise."; ::munmap(shm, size); @@ -1119,13 +1123,13 @@ d->recoverCorruptedCache(); if (!d->shm) { - kWarning(s_area) << "Lost the connection to shared memory for cache" + kWarning(ksdcArea()) << "Lost the connection to shared memory for cache" << d->m_cacheName; return false; } if (lockCount++ > 4) { - kError(s_area) << "There is a very serious problem with the KDE data cache" + kError(ksdcArea()) << "There is a very serious problem with the KDE data cache" << d->m_cacheName << "giving up trying to access cache."; ::munmap(d->shm, d->m_mapSize); d->shm = 0; @@ -1149,7 +1153,7 @@ // A while loop? Indeed, think what happens if this happens // twice -- hard to debug race conditions. while (testSize > d->m_mapSize) { - kDebug(s_area) << "Someone enlarged the cache on us," + kDebug(ksdcArea()) << "Someone enlarged the cache on us," << "attempting to match new configuration."; // Protect against two threads accessing this same KSDC @@ -1173,7 +1177,7 @@ QFile f(d->m_cacheName); if (!f.open(QFile::ReadWrite)) { - kError(s_area) << "Unable to re-open cache, unfortunately" + kError(ksdcArea()) << "Unable to re-open cache, unfortunately" << "the connection had to be dropped for" << "crash safety -- things will be much" << "slower now."; @@ -1183,7 +1187,7 @@ void *newMap = ::mmap(0, testSize, PROT_READ | PROT_WRITE, MAP_SHARED, f.handle(), 0); if (newMap == MAP_FAILED) { - kError(s_area) << "Unopen to re-map the cache into memory" + kError(ksdcArea()) << "Unopen to re-map the cache into memory" << "things will be much slower now"; return; } @@ -1233,7 +1237,7 @@ IndexTableEntry *entriesIndex = indexTable(); if (entriesIndex[index].firstPage < 0) { - kError(s_area) << "Trying to remove an entry which is already invalid. This " + kError(ksdcArea()) << "Trying to remove an entry which is already invalid. This " << "cache is likely corrupt."; clearInternalTables(); // The nuclear option... @@ -1243,14 +1247,14 @@ // Update page table first pageID firstPage = entriesIndex[index].firstPage; if (firstPage < 0 || static_cast(firstPage) >= pageTableSize()) { - kError(s_area) << "Removing" << index << "which is already marked as empty!"; + kError(ksdcArea()) << "Removing" << index << "which is already marked as empty!"; clearInternalTables(); return; } if (index != static_cast(pageTableEntries[firstPage].index)) { - kError(s_area) << "Removing" << index << "will not work as it is assigned" + kError(ksdcArea()) << "Removing" << index << "will not work as it is assigned" << "to page" << firstPage << "which is itself assigned to" << "entry" << pageTableEntries[firstPage].index << "instead!"; @@ -1268,7 +1272,7 @@ } if ((cacheAvail - savedCacheSize) != entriesToRemove) { - kError(s_area) << "We somehow did not remove" << entriesToRemove + kError(ksdcArea()) << "We somehow did not remove" << entriesToRemove << "when removing entry" << index << ", instead we removed" << (cacheAvail - savedCacheSize); } @@ -1366,7 +1370,7 @@ if (cullCollisions && (::time(0) - indices[position].lastUsedTime) > 60) { indices[position].useCount >>= 1; if (indices[position].useCount == 0) { - kDebug(s_area) << "Overwriting existing old cached entry due to collision."; + kDebug(ksdcArea()) << "Overwriting existing old cached entry due to collision."; d->shm->removeEntry(position); // Remove it first break; @@ -1379,7 +1383,7 @@ } if (indices[position].useCount > 0 && indices[position].firstPage >= 0) { - kDebug(s_area) << "Overwriting existing cached entry due to collision."; + kDebug(ksdcArea()) << "Overwriting existing cached entry due to collision."; d->shm->removeEntry(position); // Remove it first } @@ -1392,7 +1396,7 @@ uint firstPage = (uint) -1; if (pagesNeeded >= d->shm->pageTableSize()) { - kWarning(s_area) << key << "is too large to be cached."; + kWarning(ksdcArea()) << key << "is too large to be cached."; return false; } @@ -1423,7 +1427,7 @@ if (firstPage >= d->shm->pageTableSize() || d->shm->cacheAvail < pagesNeeded) { - kError(s_area) << "Unable to free up memory for" << key; + kError(ksdcArea()) << "Unable to free up memory for" << key; return false; } } @@ -1520,7 +1524,7 @@ // Note that it is important to simply unlink the file, and not truncate it // smaller first to avoid SIGBUS errors and similar with shared memory // attached to the underlying inode. - kDebug(s_area) << "Removing cache at" << cachePath; + kDebug(ksdcArea()) << "Removing cache at" << cachePath; QFile::remove(cachePath); } --- trunk/KDE/kdelibs/kdecore/util/kshareddatacache_p.h #1213835:1213836 @@ -30,7 +30,7 @@ #include // Our debug area, disabled by default -static int s_area = KDebug::registerArea("KSharedDataCache", false); +int ksdcArea(); // Mac OS X, for all its POSIX compliance, does not support timeouts on its // mutexes, which is kind of a disaster for cross-process support. However @@ -386,7 +386,7 @@ #endif default: - kError(s_area) << "Creating shell of a lock!"; + kError(ksdcArea()) << "Creating shell of a lock!"; return new KSDCLock; } }