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/chdb.cpp

@@ -18,6 +18,23 @@ static std::mutex CHDB_MUTEX;
 chdb_conn * global_conn_ptr = nullptr;
 std::string global_db_path;
 
+static void cleanUpLocalServer(DB::LocalServer *& server)
+{
+    try
+    {
+        if (server)
+        {
+            server->chdbCleanup();
+            delete server;
+            server = nullptr;
+        }
+    }
+    catch (...)
+    {
+        LOG_ERROR(&Poco::Logger::get("LocalServer"), "Error during server cleanup");
+    }
+}
+
 static DB::LocalServer * bgClickHouseLocal(int argc, char ** argv)
 {
     DB::LocalServer * app = nullptr;
@@ -31,30 +48,29 @@ static DB::LocalServer * bgClickHouseLocal(int argc, char ** argv)
         {
             auto err_msg = app->getErrorMsg();
             LOG_ERROR(&app->logger(), "Error running bgClickHouseLocal: {}", err_msg);
-            delete app;
-            app = nullptr;
+            cleanUpLocalServer(app);
             throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, "Error running bgClickHouseLocal: {}", err_msg);
         }
         return app;
     }
     catch (const DB::Exception & e)
     {
-        delete app;
+        cleanUpLocalServer(app);
         throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, "bgClickHouseLocal {}", DB::getExceptionMessage(e, false));
     }
     catch (const Poco::Exception & e)
     {
-        delete app;
+        cleanUpLocalServer(app);
         throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, "bgClickHouseLocal {}", e.displayText());
     }
     catch (const std::exception & e)
     {
-        delete app;
+        cleanUpLocalServer(app);
         throw std::domain_error(e.what());
     }
     catch (...)
     {
-        delete app;
+        cleanUpLocalServer(app);
         throw std::domain_error(DB::getCurrentExceptionMessage(true));
     }
 }
@@ -545,9 +561,10 @@ chdb_conn ** connect_chdb(int argc, char ** argv)
         [&]()
         {
             auto * queue = static_cast<CHDB::QueryQueue *>(conn->queue);
+            DB::LocalServer * server = nullptr;
             try
             {
-                DB::LocalServer * server = bgClickHouseLocal(argc, argv);
+                server = bgClickHouseLocal(argc, argv);
                 conn->server = server;
                 conn->connected = true;
 
@@ -570,16 +587,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");
-                            }
+                            cleanUpLocalServer(server);
                             queue->cleanup_done = true;
                             queue->query_cv.notify_all();
                             break;

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