diff --git a/src/Common/Arena.h b/src/Common/Arena.h index 5febed8ddf6..f1d42e53345 100644 --- a/src/Common/Arena.h +++ b/src/Common/Arena.h @@ -204,6 +204,13 @@ public: char * allocContinue(size_t additional_bytes, char const *& range_start, size_t start_alignment = 0) { + /* + * Allocating zero bytes doesn't make much sense. Also, a zero-sized + * range might break the invariant that the range begins at least before + * the current chunk end. + */ + assert(additional_bytes > 0); + if (!range_start) { // Start a new memory range. @@ -298,6 +305,9 @@ public: return size_in_bytes; } + /// Bad method, don't use it -- the chunks are not your business, the entire + /// purpose of the arena code is to manage them for you, so if you find + /// yourself having to use this method, probably you're doing something wrong. size_t remainingSpaceInCurrentChunk() const { return head->remaining(); diff --git a/src/IO/WriteBufferFromArena.h b/src/IO/WriteBufferFromArena.h index e310b6fc2ff..ee09240e9c7 100644 --- a/src/IO/WriteBufferFromArena.h +++ b/src/IO/WriteBufferFromArena.h @@ -23,7 +23,19 @@ private: { /// Allocate more memory. At least same size as used before (this gives 2x growth ratio), /// and at most grab all remaining size in current chunk of arena. - size_t continuation_size = std::max(count(), arena.remainingSpaceInCurrentChunk()); + /// + /// FIXME this class just doesn't make sense -- WriteBuffer is not + /// a unified interface for everything, it doesn't work well with + /// Arena::allocContinue -- we lose the size of data and then use a + /// heuristic to guess it back? and make a virtual call while we're at it? + /// I don't even.. + /// Being so ill-defined as it is, no wonder that the following line had + /// a bug leading to a very rare infinite loop. Just hack around it in + /// the most stupid way possible, because the real fix for this is to + /// tear down the entire WriteBuffer thing and implement it again, + /// properly. + size_t continuation_size = std::max(size_t(1), + std::max(count(), arena.remainingSpaceInCurrentChunk())); /// allocContinue method will possibly move memory region to new place and modify "begin" pointer.