From 28a3aece60a87d9044738a230716c56bc0c45888 Mon Sep 17 00:00:00 2001
From: kssenii <sumarokovakseniia@mail.ru>
Date: Tue, 8 Mar 2022 10:58:37 +0100
Subject: [PATCH] Fix uncaught exception from destructor

---
 src/Common/FileSegment.cpp                    | 19 +++++++++++---
 src/Common/FileSegment.h                      | 14 +++++++++-
 src/Disks/IO/CachedReadBufferFromRemoteFS.cpp | 26 +++++++++----------
 3 files changed, 42 insertions(+), 17 deletions(-)

diff --git a/src/Common/FileSegment.cpp b/src/Common/FileSegment.cpp
index 291ce2ca86f..bab67f546bf 100644
--- a/src/Common/FileSegment.cpp
+++ b/src/Common/FileSegment.cpp
@@ -51,8 +51,21 @@ size_t FileSegment::getDownloadOffset() const
 
 String FileSegment::getCallerId()
 {
-    if (!CurrentThread::isInitialized() || CurrentThread::getQueryId().size == 0)
+    return getCallerIdImpl(false);
+}
+
+String FileSegment::getCallerIdImpl(bool allow_non_strict_checking)
+{
+    if (IFileCache::shouldBypassCache())
+    {
+        /// getCallerId() can be called from completeImpl(), which can be called from complete().
+        /// complete() is called from destructor of CachedReadBufferFromRemoteFS when there is no query id anymore.
+        /// Allow non strict checking only for internal usage.
+        if (allow_non_strict_checking)
+            return "None";
+
         throw Exception(ErrorCodes::REMOTE_FS_OBJECT_CACHE_ERROR, "Cannot use cache without query id");
+    }
 
     return CurrentThread::getQueryId().toString() + ":" + toString(getThreadId());
 }
@@ -321,7 +334,7 @@ void FileSegment::complete()
     cv.notify_all();
 }
 
-void FileSegment::completeImpl()
+void FileSegment::completeImpl(bool allow_non_strict_checking)
 {
     /// cache lock is always taken before segment lock.
     std::lock_guard cache_lock(cache->mutex);
@@ -361,7 +374,7 @@ void FileSegment::completeImpl()
         }
     }
 
-    if (downloader_id == getCallerId())
+    if (!downloader_id.empty() && downloader_id == getCallerIdImpl(allow_non_strict_checking))
     {
         LOG_TEST(log, "Clearing downloader id: {}, current state: {}", downloader_id, stateToString(download_state));
         downloader_id.clear();
diff --git a/src/Common/FileSegment.h b/src/Common/FileSegment.h
index 473f2cfbc03..7c3520aebf7 100644
--- a/src/Common/FileSegment.h
+++ b/src/Common/FileSegment.h
@@ -125,8 +125,9 @@ private:
     size_t availableSize() const { return reserved_size - downloaded_size; }
     bool lastFileSegmentHolder() const;
     void complete();
-    void completeImpl();
+    void completeImpl(bool allow_non_strict_checking = false);
     void setDownloaded(std::lock_guard<std::mutex> & segment_lock);
+    static String getCallerIdImpl(bool allow_non_strict_checking = false);
 
     const Range segment_range;
 
@@ -164,7 +165,18 @@ struct FileSegmentsHolder : private boost::noncopyable
         /// remain only uncompleted file segments.
 
         for (auto & segment : file_segments)
+        {
             segment->complete();
+
+            // try
+            // {
+            //     segment->complete();
+            // }
+            // catch (...)
+            // {
+            //     tryLogCurrentException(__PRETTY_FUNCTION__);
+            // }
+        }
     }
 
     FileSegments file_segments{};
diff --git a/src/Disks/IO/CachedReadBufferFromRemoteFS.cpp b/src/Disks/IO/CachedReadBufferFromRemoteFS.cpp
index af988498129..cafd2b68eba 100644
--- a/src/Disks/IO/CachedReadBufferFromRemoteFS.cpp
+++ b/src/Disks/IO/CachedReadBufferFromRemoteFS.cpp
@@ -491,22 +491,22 @@ bool CachedReadBufferFromRemoteFS::nextImpl()
     if (current_file_segment_it == file_segments_holder->file_segments.end())
         return false;
 
-    // SCOPE_EXIT({
-    //     if (current_file_segment_it == file_segments_holder->file_segments.end())
-    //         return;
+    SCOPE_EXIT({
+        if (current_file_segment_it == file_segments_holder->file_segments.end())
+            return;
 
-    //     auto & file_segment = *current_file_segment_it;
+        auto & file_segment = *current_file_segment_it;
 
-    //     bool download_current_segment = read_type == ReadType::REMOTE_FS_READ_AND_PUT_IN_CACHE;
-    //     if (download_current_segment)
-    //     {
-    //         bool file_segment_already_completed = !file_segment->isDownloader();
-    //         if (!file_segment_already_completed)
-    //             file_segment->completeBatchAndResetDownloader();
-    //     }
+        bool download_current_segment = read_type == ReadType::REMOTE_FS_READ_AND_PUT_IN_CACHE;
+        if (download_current_segment)
+        {
+            bool file_segment_already_completed = !file_segment->isDownloader();
+            if (!file_segment_already_completed)
+                file_segment->completeBatchAndResetDownloader();
+        }
 
-    //     assert(!file_segment->isDownloader());
-    // });
+        assert(!file_segment->isDownloader());
+    });
 
     bytes_to_predownload = 0;