From 953d9e82c7bff02de945108a00e5acc455929102 Mon Sep 17 00:00:00 2001
From: web3-developer <51288821+web3-developer@users.noreply.github.com>
Date: Thu, 27 Jun 2024 10:22:12 +0800
Subject: [PATCH] Revert some changes from prior PR
 https://github.com/status-im/nim-rocksdb/pull/48 which enable memory leak of
 options when not cleaned up manually.

---
 rocksdb/rocksdb.nim       | 27 +++++++--------------------
 rocksdb/sstfilewriter.nim |  4 ++--
 rocksdb/transactiondb.nim | 25 +++++--------------------
 tests/test_helper.nim     |  1 -
 4 files changed, 14 insertions(+), 43 deletions(-)

diff --git a/rocksdb/rocksdb.nim b/rocksdb/rocksdb.nim
index d012494..dbfe8c3 100644
--- a/rocksdb/rocksdb.nim
+++ b/rocksdb/rocksdb.nim
@@ -103,9 +103,9 @@ proc listColumnFamilies*(
 
 proc openRocksDb*(
     path: string,
-    dbOpts = DbOptionsRef(nil),
-    readOpts = ReadOptionsRef(nil),
-    writeOpts = WriteOptionsRef(nil),
+    dbOpts = defaultDbOptions(),
+    readOpts = defaultReadOptions(),
+    writeOpts = defaultWriteOptions(),
     columnFamilies: openArray[ColFamilyDescriptor] = [],
 ): RocksDBResult[RocksDbReadWriteRef] =
   ## Open a RocksDB instance in read-write mode. If `columnFamilies` is empty
@@ -114,10 +114,6 @@ proc openRocksDb*(
   ## By default, column families will be created if they don't yet exist.
   ## All existing column families must be specified if the database has
   ## previously created any column families.
-  let useDbOpts = (if dbOpts.isNil: defaultDbOptions() else: dbOpts)
-  defer:
-    if dbOpts.isNil:
-      useDbOpts.close()
 
   var cfs = columnFamilies.toSeq()
   if DEFAULT_COLUMN_FAMILY_NAME notin columnFamilies.mapIt(it.name()):
@@ -129,7 +125,7 @@ proc openRocksDb*(
     cfHandles = newSeq[ColFamilyHandlePtr](cfs.len)
     errors: cstring
   let rocksDbPtr = rocksdb_open_column_families(
-    useDbOpts.cPtr,
+    dbOpts.cPtr,
     path.cstring,
     cfNames.len().cint,
     cast[cstringArray](cfNames[0].addr),
@@ -140,9 +136,6 @@ proc openRocksDb*(
   bailOnErrors(errors)
 
   let
-    dbOpts = useDbOpts # don't close on exit
-    readOpts = (if readOpts.isNil: defaultReadOptions() else: readOpts)
-    writeOpts = (if writeOpts.isNil: defaultWriteOptions() else: writeOpts)
     cfTable = newColFamilyTable(cfNames.mapIt($it), cfHandles)
     db = RocksDbReadWriteRef(
       lock: createLock(),
@@ -159,8 +152,8 @@ proc openRocksDb*(
 
 proc openRocksDbReadOnly*(
     path: string,
-    dbOpts = DbOptionsRef(nil),
-    readOpts = ReadOptionsRef(nil),
+    dbOpts = defaultDbOptions(),
+    readOpts = defaultReadOptions(),
     columnFamilies: openArray[ColFamilyDescriptor] = [],
     errorIfWalFileExists = false,
 ): RocksDBResult[RocksDbReadOnlyRef] =
@@ -170,10 +163,6 @@ proc openRocksDbReadOnly*(
   ## families will be created if they don't yet exist. If the database already
   ## contains any column families, then all or a subset of the existing column
   ## families can be opened for reading.
-  let useDbOpts = (if dbOpts.isNil: defaultDbOptions() else: dbOpts)
-  defer:
-    if dbOpts.isNil:
-      useDbOpts.close()
 
   var cfs = columnFamilies.toSeq()
   if DEFAULT_COLUMN_FAMILY_NAME notin columnFamilies.mapIt(it.name()):
@@ -185,7 +174,7 @@ proc openRocksDbReadOnly*(
     cfHandles = newSeq[ColFamilyHandlePtr](cfs.len)
     errors: cstring
   let rocksDbPtr = rocksdb_open_for_read_only_column_families(
-    useDbOpts.cPtr,
+    dbOpts.cPtr,
     path.cstring,
     cfNames.len().cint,
     cast[cstringArray](cfNames[0].addr),
@@ -197,8 +186,6 @@ proc openRocksDbReadOnly*(
   bailOnErrors(errors)
 
   let
-    dbOpts = useDbOpts # don't close on exit
-    readOpts = (if readOpts.isNil: defaultReadOptions() else: readOpts)
     cfTable = newColFamilyTable(cfNames.mapIt($it), cfHandles)
     db = RocksDbReadOnlyRef(
       lock: createLock(),
diff --git a/rocksdb/sstfilewriter.nim b/rocksdb/sstfilewriter.nim
index 63b4b9b..eaf77e9 100644
--- a/rocksdb/sstfilewriter.nim
+++ b/rocksdb/sstfilewriter.nim
@@ -25,10 +25,9 @@ type
     dbOpts: DbOptionsRef
 
 proc openSstFileWriter*(
-    filePath: string, dbOpts = DbOptionsRef(nil)
+    filePath: string, dbOpts = defaultDbOptions()
 ): RocksDBResult[SstFileWriterRef] =
   ## Creates a new `SstFileWriterRef` and opens the file at the given `filePath`.
-  let dbOpts = (if dbOpts.isNil: defaultDbOptions() else: dbOpts)
   doAssert not dbOpts.isClosed()
 
   let envOptsPtr = rocksdb_envoptions_create()
@@ -95,6 +94,7 @@ proc finish*(writer: SstFileWriterRef): RocksDBResult[void] =
 proc close*(writer: SstFileWriterRef) =
   ## Closes the `SstFileWriterRef`.
   if not writer.isClosed():
+    writer.dbOpts.close()
     rocksdb_envoptions_destroy(writer.envOptsPtr)
     writer.envOptsPtr = nil
     rocksdb_sstfilewriter_destroy(writer.cPtr)
diff --git a/rocksdb/transactiondb.nim b/rocksdb/transactiondb.nim
index 71f4dfc..35d0efa 100644
--- a/rocksdb/transactiondb.nim
+++ b/rocksdb/transactiondb.nim
@@ -41,17 +41,13 @@ type
 
 proc openTransactionDb*(
     path: string,
-    dbOpts = DbOptionsRef(nil),
-    txDbOpts = TransactionDbOptionsRef(nil),
+    dbOpts = defaultDbOptions(),
+    txDbOpts = defaultTransactionDbOptions(),
     columnFamilies: openArray[ColFamilyDescriptor] = [],
 ): RocksDBResult[TransactionDbRef] =
   ## Open a `TransactionDbRef` with the given options and column families.
   ## If no column families are provided the default column family will be used.
   ## If no options are provided the default options will be used.
-  let useDbOpts = (if dbOpts.isNil: defaultDbOptions() else: dbOpts)
-  defer:
-    if dbOpts.isNil:
-      useDbOpts.close()
 
   var cfs = columnFamilies.toSeq()
   if DEFAULT_COLUMN_FAMILY_NAME notin columnFamilies.mapIt(it.name()):
@@ -64,7 +60,7 @@ proc openTransactionDb*(
     errors: cstring
 
   let txDbPtr = rocksdb_transactiondb_open_column_families(
-    useDbOpts.cPtr,
+    dbOpts.cPtr,
     txDbOpts.cPtr,
     path.cstring,
     cfNames.len().cint,
@@ -76,10 +72,6 @@ proc openTransactionDb*(
   bailOnErrors(errors)
 
   let
-    dbOpts = useDbOpts # don't close on exit
-    txDbOpts = (if txDbOpts.isNil: defaultTransactionDbOptions()
-    else: txDbOpts
-    )
     cfTable = newColFamilyTable(cfNames.mapIt($it), cfHandles)
     db = TransactionDbRef(
       lock: createLock(),
@@ -107,9 +99,8 @@ proc isClosed*(db: TransactionDbRef): bool {.inline.} =
 
 proc beginTransaction*(
     db: TransactionDbRef,
-    readOpts = ReadOptionsRef(nil),
-    writeOpts = WriteOptionsRef(nil),
-    txDbOpts = TransactionDbOptionsRef(nil),
+    readOpts = defaultReadOptions(),
+    writeOpts = defaultWriteOptions(),
     txOpts = defaultTransactionOptions(),
     cfHandle = db.defaultCfHandle,
 ): TransactionRef =
@@ -117,12 +108,6 @@ proc beginTransaction*(
   ## to using the specified column family. If no column family is specified
   ## then the default column family will be used.
   doAssert not db.isClosed()
-  let
-    txDbOpts = (if txDbOpts.isNil: defaultTransactionDbOptions()
-    else: txDbOpts
-    )
-    readOpts = (if readOpts.isNil: defaultReadOptions() else: readOpts)
-    writeOpts = (if writeOpts.isNil: defaultWriteOptions() else: writeOpts)
 
   let txPtr = rocksdb_transaction_begin(db.cPtr, writeOpts.cPtr, txOpts.cPtr, nil)
 
diff --git a/tests/test_helper.nim b/tests/test_helper.nim
index b0eae61..87196fa 100644
--- a/tests/test_helper.nim
+++ b/tests/test_helper.nim
@@ -43,7 +43,6 @@ proc initTransactionDb*(
 ): TransactionDbRef =
   let res = openTransactionDb(
     path,
-    txDbOpts = defaultTransactionDbOptions(),
     columnFamilies = columnFamilyNames.mapIt(initColFamilyDescriptor(it)),
   )
   if res.isErr():