Reverted trunk r12255 changes. Provided a portable flexible arrays replacement. Trunk r12255 made Clang compiler happy by removing flexible nonPod[] arrays. Unfortunately, it also moved shared memory items into local memory (in some cases uninitialized). This change provides a Clang-friendly flexible array replacement while keeping items in the shared memory (and using placement-new initialization). The code may have become even less readable, but hopefully more portable. N.B. Flexible arrays were introdiced in C99 standard, after C++ was standardized in 1998. They are not yet in any revised C++ standard, but most C++ compilers support them, at least for PODs. === modified file 'src/ipc/Makefile.am' --- src/ipc/Makefile.am 2011-10-28 01:01:41 +0000 +++ src/ipc/Makefile.am 2012-10-12 23:48:24 +0000 @@ -29,36 +29,37 @@ SharedListen.cc \ SharedListen.h \ TypedMsgHdr.cc \ TypedMsgHdr.h \ Coordinator.cc \ Coordinator.h \ UdsOp.cc \ UdsOp.h \ Port.cc \ Port.h \ Strand.cc \ Strand.h \ forward.h \ Forwarder.cc \ Forwarder.h \ Inquirer.cc \ Inquirer.h \ Request.h \ Response.h \ \ + mem/FlexibleArray.h \ mem/Page.cc \ mem/Page.h \ mem/PagePool.cc \ mem/PagePool.h \ mem/Pages.cc \ mem/Pages.h \ mem/PageStack.cc \ mem/PageStack.h \ mem/Pointer.h \ mem/Segment.cc \ mem/Segment.h DEFS += -DDEFAULT_STATEDIR=\"$(localstatedir)/run/squid\" install-data-local: $(mkinstalldirs) $(DESTDIR)$(localstatedir)/run/squid; === modified file 'src/ipc/Queue.cc' --- src/ipc/Queue.cc 2012-09-01 14:38:36 +0000 +++ src/ipc/Queue.cc 2012-10-12 23:50:06 +0000 @@ -28,49 +28,44 @@ /// constructs QueueReaders ID from parent queue ID static String ReadersId(String id) { id.append("__readers"); return id; } /* QueueReader */ InstanceIdDefinitions(Ipc::QueueReader, "ipcQR"); Ipc::QueueReader::QueueReader(): popBlocked(1), popSignal(0), rateLimit(0), balance(0) { debugs(54, 7, HERE << "constructed " << id); } /* QueueReaders */ -Ipc::QueueReaders::QueueReaders(const int aCapacity): theCapacity(aCapacity) +Ipc::QueueReaders::QueueReaders(const int aCapacity): theCapacity(aCapacity), + theReaders(theCapacity) { Must(theCapacity > 0); - theReaders=new QueueReader[theCapacity]; -} - -Ipc::QueueReaders::~QueueReaders() -{ - delete[] theReaders; } size_t Ipc::QueueReaders::sharedMemorySize() const { return SharedMemorySize(theCapacity); } size_t Ipc::QueueReaders::SharedMemorySize(const int capacity) { return sizeof(QueueReaders) + sizeof(QueueReader) * capacity; } // OneToOneUniQueue Ipc::OneToOneUniQueue::OneToOneUniQueue(const unsigned int aMaxItemSize, const int aCapacity): theIn(0), theOut(0), theSize(0), theMaxItemSize(aMaxItemSize), theCapacity(aCapacity) { === modified file 'src/ipc/Queue.h' --- src/ipc/Queue.h 2012-09-01 14:38:36 +0000 +++ src/ipc/Queue.h 2012-10-12 22:46:53 +0000 @@ -1,30 +1,31 @@ /* */ #ifndef SQUID_IPC_QUEUE_H #define SQUID_IPC_QUEUE_H #include "Array.h" #include "Debug.h" #include "base/InstanceId.h" #include "ipc/AtomicWord.h" +#include "ipc/mem/FlexibleArray.h" #include "ipc/mem/Pointer.h" #include "util.h" class String; namespace Ipc { /// State of the reading end of a queue (i.e., of the code calling pop()). /// Multiple queues attached to one reader share this state. class QueueReader { public: QueueReader(); // the initial state is "blocked without a signal" /// whether the reader is waiting for a notification signal bool blocked() const { return popBlocked == 1; } /// marks the reader as blocked, waiting for a notification signal void block() { popBlocked.swap_if(0, 1); } @@ -45,50 +46,45 @@ public: typedef Atomic::Word Rate; ///< pop()s per second Rate rateLimit; ///< pop()s per second limit if positive // we need a signed atomic type because balance may get negative typedef Atomic::WordT AtomicSignedMsec; typedef AtomicSignedMsec Balance; /// how far ahead the reader is compared to a perfect read/sec event rate Balance balance; /// unique ID for debugging which reader is used (works across processes) const InstanceId id; }; /// shared array of QueueReaders class QueueReaders { public: QueueReaders(const int aCapacity); - ~QueueReaders(); size_t sharedMemorySize() const; static size_t SharedMemorySize(const int capacity); const int theCapacity; /// number of readers - QueueReader *theReaders; /// readers -private: - QueueReaders(); //not implemented - QueueReaders& operator =(const QueueReaders&); //not implemented - QueueReaders(const QueueReaders&); //not implemented + Ipc::Mem::FlexibleArray theReaders; /// readers }; /** * Lockless fixed-capacity queue for a single writer and a single reader. * * If the queue is empty, the reader is considered "blocked" and needs * an out-of-band notification message to notice the next pushed item. * * Current implementation assumes that the writer cannot get blocked: if the * queue is full, the writer will just not push and come back later (with a * different value). We can add support for blocked writers if needed. */ class OneToOneUniQueue { public: // pop() and push() exceptions; TODO: use TextException instead class Full {}; class ItemTooLarge {}; OneToOneUniQueue(const unsigned int aMaxItemSize, const int aCapacity); === modified file 'src/ipc/StoreMap.cc' --- src/ipc/StoreMap.cc 2012-08-31 16:57:39 +0000 +++ src/ipc/StoreMap.cc 2012-10-12 23:45:46 +0000 @@ -238,48 +238,48 @@ int Ipc::StoreMap::slotIndexByKey(const cache_key *const key) const { const uint64_t *const k = reinterpret_cast(key); // TODO: use a better hash function return (k[0] + k[1]) % shared->limit; } Ipc::StoreMap::Slot & Ipc::StoreMap::slotByKey(const cache_key *const key) { return shared->slots[slotIndexByKey(key)]; } /// unconditionally frees the already exclusively locked slot and releases lock void Ipc::StoreMap::freeLocked(Slot &s, bool keepLocked) { if (s.state == Slot::Readable && cleaner) - cleaner->cleanReadable(&s - shared->slots); + cleaner->cleanReadable(&s - shared->slots.raw()); s.waitingToBeFreed = false; s.state = Slot::Empty; if (!keepLocked) s.lock.unlockExclusive(); --shared->count; - debugs(54, 5, HERE << " freed slot at " << (&s - shared->slots) << + debugs(54, 5, HERE << " freed slot at " << (&s - shared->slots.raw()) << " in map [" << path << ']'); } /* Ipc::StoreMapSlot */ Ipc::StoreMapSlot::StoreMapSlot(): state(Empty) { xmemset(&key, 0, sizeof(key)); xmemset(&basics, 0, sizeof(basics)); } void Ipc::StoreMapSlot::setKey(const cache_key *const aKey) { memcpy(key, aKey, sizeof(key)); } bool Ipc::StoreMapSlot::sameKey(const cache_key *const aKey) const { @@ -287,42 +287,36 @@ return k[0] == key[0] && k[1] == key[1]; } void Ipc::StoreMapSlot::set(const StoreEntry &from) { memcpy(key, from.key, sizeof(key)); // XXX: header = aHeader; basics.timestamp = from.timestamp; basics.lastref = from.lastref; basics.expires = from.expires; basics.lastmod = from.lastmod; basics.swap_file_sz = from.swap_file_sz; basics.refcount = from.refcount; basics.flags = from.flags; } /* Ipc::StoreMap::Shared */ Ipc::StoreMap::Shared::Shared(const int aLimit, const size_t anExtrasSize): - limit(aLimit), extrasSize(anExtrasSize), count(0) + limit(aLimit), extrasSize(anExtrasSize), count(0), slots(aLimit) { - slots=new Slot[limit]; -} - -Ipc::StoreMap::Shared::~Shared() -{ - delete[] slots; } size_t Ipc::StoreMap::Shared::sharedMemorySize() const { return SharedMemorySize(limit, extrasSize); } size_t Ipc::StoreMap::Shared::SharedMemorySize(const int limit, const size_t extrasSize) { return sizeof(Shared) + limit * (sizeof(Slot) + extrasSize); } === modified file 'src/ipc/StoreMap.h' --- src/ipc/StoreMap.h 2012-08-28 13:00:30 +0000 +++ src/ipc/StoreMap.h 2012-10-12 23:39:58 +0000 @@ -1,24 +1,25 @@ #ifndef SQUID_IPC_STORE_MAP_H #define SQUID_IPC_STORE_MAP_H #include "ipc/ReadWriteLock.h" +#include "ipc/mem/FlexibleArray.h" #include "ipc/mem/Pointer.h" #include "typedefs.h" namespace Ipc { /// a StoreMap element, holding basic shareable StoreEntry info class StoreMapSlot { public: StoreMapSlot(); /// store StoreEntry key and basics void set(const StoreEntry &anEntry); void setKey(const cache_key *const aKey); bool sameKey(const cache_key *const aKey) const; public: mutable ReadWriteLock lock; ///< protects slot data below @@ -45,50 +46,45 @@ } State; State state; ///< current state }; class StoreMapCleaner; /// map of StoreMapSlots indexed by their keys, with read/write slot locking /// kids extend to store custom data class StoreMap { public: typedef StoreMapSlot Slot; /// data shared across maps in different processes class Shared { public: Shared(const int aLimit, const size_t anExtrasSize); size_t sharedMemorySize() const; static size_t SharedMemorySize(const int limit, const size_t anExtrasSize); - ~Shared(); const int limit; ///< maximum number of map slots const size_t extrasSize; ///< size of slot extra data Atomic::Word count; ///< current number of map slots - Slot *slots; ///< slots storage - private: - Shared(); //disabled - Shared &operator=(const Shared&); //disabled - Shared(const Shared&); //disabled + Ipc::Mem::FlexibleArray slots; ///< slots storage }; public: typedef Mem::Owner Owner; /// initialize shared memory static Owner *Init(const char *const path, const int limit); StoreMap(const char *const aPath); /// finds, reservers space for writing a new entry or returns nil Slot *openForWriting(const cache_key *const key, sfileno &fileno); /// successfully finish writing the entry void closeForWriting(const sfileno fileno, bool lockForReading = false); /// only works on locked entries; returns nil unless the slot is readable const Slot *peekAtReader(const sfileno fileno) const; /// mark the slot as waiting to be freed and, if possible, free it void free(const sfileno fileno); === added file 'src/ipc/mem/FlexibleArray.h' --- src/ipc/mem/FlexibleArray.h 1970-01-01 00:00:00 +0000 +++ src/ipc/mem/FlexibleArray.h 2012-10-13 05:12:20 +0000 @@ -0,0 +1,45 @@ +/* + */ + +#ifndef SQUID_IPC_MEM_FLEXIBLE_ARRAY_H +#define SQUID_IPC_MEM_FLEXIBLE_ARRAY_H + +// sometimes required for placement-new operator to be declared +#include + +namespace Ipc +{ + +namespace Mem +{ + +/// A "flexible array" of Items inside some shared memory space. +/// A portable equivalent of a "Item items[];" data member. +/// Some compilers such as Clang can only handle flexible arrays of PODs, +/// and the current C++ standard does not allow flexible arrays at all. +template +class FlexibleArray +{ +public: + explicit FlexibleArray(const int capacity) { + if (capacity > 1) // the first item is initialized automatically + new (items+1) Item[capacity-1]; + } + + Item &operator [](const int idx) { return items[idx]; } + const Item &operator [](const int idx) const { return items[idx]; } + + //const Item *operator ()() const { return items; } + //Item *operator ()() { return items; } + + Item *raw() { return items; } + +private: + Item items[1]; // ensures proper alignment of array elements +}; + +} // namespace Mem + +} // namespace Ipc + +#endif /* SQUID_IPC_MEM_FLEXIBLE_ARRAY_H */ === modified file 'src/ipc/mem/PageStack.cc' --- src/ipc/mem/PageStack.cc 2012-10-03 10:18:42 +0000 +++ src/ipc/mem/PageStack.cc 2012-10-13 05:12:31 +0000 @@ -1,50 +1,45 @@ /* * DEBUG: section 54 Interprocess Communication * */ #include "squid.h" #include "base/TextException.h" #include "ipc/mem/Page.h" #include "ipc/mem/PageStack.h" /// used to mark a stack slot available for storing free page offsets const Ipc::Mem::PageStack::Value Writable = 0; Ipc::Mem::PageStack::PageStack(const uint32_t aPoolId, const unsigned int aCapacity, const size_t aPageSize): thePoolId(aPoolId), theCapacity(aCapacity), thePageSize(aPageSize), theSize(theCapacity), - theLastReadable(prev(theSize)), theFirstWritable(next(theLastReadable)) + theLastReadable(prev(theSize)), theFirstWritable(next(theLastReadable)), + theItems(aCapacity) { - theItems=new Item[theSize]; // initially, all pages are free for (Offset i = 0; i < theSize; ++i) theItems[i] = i + 1; // skip page number zero to keep numbers positive } -Ipc::Mem::PageStack::~PageStack() -{ - delete[] theItems; -} - /* * TODO: We currently rely on the theLastReadable hint during each * loop iteration. We could also use hint just for the start position: * (const Offset start = theLastReadable) and then scan the stack * sequentially regardless of theLastReadable changes by others. Which * approach is better? Same for push(). */ bool Ipc::Mem::PageStack::pop(PageId &page) { Must(!page); // we may fail to dequeue, but be conservative to prevent long searches --theSize; // find a Readable slot, starting with theLastReadable and going left while (theSize >= 0) { const Offset idx = theLastReadable; // mark the slot at ids Writable while extracting its current value const Value value = theItems[idx].fetchAndAnd(0); // works if Writable is 0 === modified file 'src/ipc/mem/PageStack.h' --- src/ipc/mem/PageStack.h 2012-09-01 14:38:36 +0000 +++ src/ipc/mem/PageStack.h 2012-10-12 23:53:22 +0000 @@ -1,46 +1,46 @@ /* */ #ifndef SQUID_IPC_MEM_PAGE_STACK_H #define SQUID_IPC_MEM_PAGE_STACK_H #include "ipc/AtomicWord.h" +#include "ipc/mem/FlexibleArray.h" namespace Ipc { namespace Mem { class PageId; /// Atomic container of "free" page numbers inside a single SharedMemory space. /// Assumptions: all page numbers are unique, positive, have an known maximum, /// and can be temporary unavailable as long as they are never trully lost. class PageStack { public: typedef uint32_t Value; ///< stack item type (a free page number) PageStack(const uint32_t aPoolId, const unsigned int aCapacity, const size_t aPageSize); - ~PageStack(); unsigned int capacity() const { return theCapacity; } size_t pageSize() const { return thePageSize; } /// lower bound for the number of free pages unsigned int size() const { return max(0, theSize.get()); } /// sets value and returns true unless no free page numbers are found bool pop(PageId &page); /// makes value available as a free page number to future pop() callers void push(PageId &page); bool pageIdIsValid(const PageId &page) const; /// total shared memory size required to share static size_t SharedMemorySize(const uint32_t aPoolId, const unsigned int capacity, const size_t pageSize); size_t sharedMemorySize() const; /// shared memory size required only by PageStack, excluding /// shared counters and page data static size_t StackSize(const unsigned int capacity); @@ -49,28 +49,28 @@ private: /// stack index and size type (may temporary go negative) typedef int Offset; // these help iterate the stack in search of a free spot or a page Offset next(const Offset idx) const { return (idx + 1) % theCapacity; } Offset prev(const Offset idx) const { return (theCapacity + idx - 1) % theCapacity; } const uint32_t thePoolId; ///< pool ID const Offset theCapacity; ///< stack capacity, i.e. theItems size const size_t thePageSize; ///< page size, used to calculate shared memory size /// lower bound for the number of free pages (may get negative!) Atomic::WordT theSize; /// last readable item index; just a hint, not a guarantee Atomic::WordT theLastReadable; /// first writable item index; just a hint, not a guarantee Atomic::WordT theFirstWritable; typedef Atomic::WordT Item; - Item *theItems; ///< page number storage + Ipc::Mem::FlexibleArray theItems; ///< page number storage }; } // namespace Mem } // namespace Ipc #endif // SQUID_IPC_MEM_PAGE_STACK_H