Merge pull request #10466 from ClickHouse/aku/arena-infinite-loop

Work around a bug leading to an infinite loop in addressToLine

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();

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.