Fix open session after a previous failure

chdb/session/state.py

@@ -40,6 +40,7 @@ class Session:
     """
 
     def __init__(self, path=None):
+        self._conn = None
         global g_session, g_session_path
         if g_session is not None:
             warnings.warn(

programs/local/LocalServer.cpp

@@ -157,6 +157,7 @@ void applySettingsOverridesForLocal(ContextMutablePtr context)
 
 LocalServer::~LocalServer()
 {
+    cleanup();
     resetQueryOutputVector();
 }
 

programs/local/LocalServer.h

@@ -89,11 +89,6 @@ class LocalServer : public ClientApplicationBase, public Loggers
         return local_connection->getCHDBProgress().read_bytes;
     }
 
-    void chdbCleanup()
-    {
-        cleanup();
-    }
-
 private:
     void cleanStreamingQuery();
 };

programs/local/chdb.cpp

@@ -18,43 +18,37 @@ static std::mutex CHDB_MUTEX;
 chdb_conn * global_conn_ptr = nullptr;
 std::string global_db_path;
 
-static DB::LocalServer * bgClickHouseLocal(int argc, char ** argv)
+static std::unique_ptr<DB::LocalServer> bgClickHouseLocal(int argc, char ** argv)
 {
-    DB::LocalServer * app = nullptr;
+    std::unique_ptr<DB::LocalServer> app;
     try
     {
-        app = new DB::LocalServer();
+        app = std::make_unique<DB::LocalServer>();
         app->setBackground(true);
         app->init(argc, argv);
         int ret = app->run();
         if (ret != 0)
         {
             auto err_msg = app->getErrorMsg();
             LOG_ERROR(&app->logger(), "Error running bgClickHouseLocal: {}", err_msg);
-            delete app;
-            app = nullptr;
             throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, "Error running bgClickHouseLocal: {}", err_msg);
         }
         return app;
     }
     catch (const DB::Exception & e)
     {
-        delete app;
         throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, "bgClickHouseLocal {}", DB::getExceptionMessage(e, false));
     }
     catch (const Poco::Exception & e)
     {
-        delete app;
         throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, "bgClickHouseLocal {}", e.displayText());
     }
     catch (const std::exception & e)
     {
-        delete app;
         throw std::domain_error(e.what());
     }
     catch (...)
     {
-        delete app;
         throw std::domain_error(DB::getCurrentExceptionMessage(true));
     }
 }
@@ -545,10 +539,11 @@ chdb_conn ** connect_chdb(int argc, char ** argv)
         [&]()
         {
             auto * queue = static_cast<CHDB::QueryQueue *>(conn->queue);
+            std::unique_ptr<DB::LocalServer> server;
             try
             {
-                DB::LocalServer * server = bgClickHouseLocal(argc, argv);
-                conn->server = server;
+                server = bgClickHouseLocal(argc, argv);
+                conn->server = nullptr;
                 conn->connected = true;
 
                 global_conn_ptr = conn;
@@ -570,16 +565,7 @@ chdb_conn ** connect_chdb(int argc, char ** argv)
 
                         if (queue->shutdown)
                         {
-                            try
-                            {
-                                server->chdbCleanup();
-                                delete server;
-                            }
-                            catch (...)
-                            {
-                                // Log error but continue shutdown
-                                LOG_ERROR(&Poco::Logger::get("LocalServer"), "Error during server cleanup");
-                            }
+                            server.reset();
                             queue->cleanup_done = true;
                             queue->query_cv.notify_all();
                             break;
@@ -588,7 +574,7 @@ chdb_conn ** connect_chdb(int argc, char ** argv)
                     }
 
                     CHDB::QueryRequestBase & req = *(queue->current_query);
-                    auto result = createQueryResult(server, req);
+                    auto result = createQueryResult(server.get(), req);
                     bool is_end = result.second;
 
                     {

tests/test_open_session_after_failure.py

@@ -0,0 +1,36 @@
+#!python3
+
+import unittest
+import shutil
+from chdb import session
+
+
+test_dir1 = ".test_open_session_after_failure"
+test_dir2 = "/usr/bin"
+
+
+class TestStateful(unittest.TestCase):
+    def setUp(self) -> None:
+        shutil.rmtree(test_dir1, ignore_errors=True)
+        return super().setUp()
+
+    def tearDown(self) -> None:
+        shutil.rmtree(test_dir1, ignore_errors=True)
+        return super().tearDown()
+
+    def test_path(self):
+        # Test that creating session with invalid path (read-only directory) raises exception
+        with self.assertRaises(Exception):
+            sess = session.Session(test_dir2)
+
+        # Test that creating session with valid path works after failure
+        sess = session.Session(test_dir1)
+
+        ret = sess.query("select 'aaaaa'")
+        self.assertEqual(str(ret), "\"aaaaa\"\n")
+
+        sess.close()
+
+
+if __name__ == '__main__':
+    unittest.main()