slOpen memory leak

[cj]

This demonstrates how multiple global handles are incorrectly created (programming error) for the current database.
Notice the database cannot close correctly or be killed if this is done.  Permission denied error.
Calling slOpen many times without closing it will eventually produce an error 7.

Code Select Expand

GLOBAL gs AS STRING
#INCLUDE "sqlitening.inc"

FUNCTION PBMAIN () AS LONG
slSetProcessMods "E2"
LOCAL x AS LONG
Logit "Open database only if not open"
FOR x = 1 TO 5
  IF ISFALSE(slIsOpen) THEN slopen "junk.tmp","C": Logit USING$("handle #",slGetHandle)
NEXT
slClose
KILL "junk.tmp"
Logit  ERROR$(ERR) + " killing junk.tmp"


[b]'Now show how to incorrectly use slOpen[/b]

Logit ""
Logit "Open database without testing if already open"
FOR x = 1 TO 5
  slopen "junk.tmp","C"
  Logit USING$("handle #",slGetHandle)
NEXT
slClose
KILL "junk.tmp"
LOGIT ERROR$(ERR) + " killing junk.tmp"

? gs

END FUNCTION

SUB LogIt(s AS STRING)
gs = gs + s + $CR
END SUB

[Bern Ertl]

Is there ever a case where opening more than one handle to the same database file within a single process might be useful?  I can't think of one right now, but maybe there is.

[cj]

Hi, Bern.

I can't think of any use.  The docs mention making direct calls to SQLite.
I only use slOpen in other threads.
.
I never realized a new global or threaded file handle was created with each call to slOpen.
I always thought SQlitening ignored a call to an already open database.

I will be modifying some programs by mostly using IF ISFALSE(slIsOpen) Then slOpen ...

slGetHandle ([rsModChars String, rlSetNumber Long]) Dword

Returns the requested handle.  ModChars will determine which handle is returned.  The database handle may be used to call SQLite directly or can be passed to a different thread to be used in slOpen.  The set handle may only be used to call SQLite directly and then only if in local mode. A %SQLitening_InvalidStringOrRequest error will occur if you try to get a set handle in remote mode.


ModChars:
·    D = Return the open database handle. This is the default.
·    S = Return the open set handle for set number passed in SetNumber.


Returns zero if an error occurs and the global return errors flag is on.

[cj]

FUNCTION slOpen ALIAS "slOpen  ...
  IF thDab THEN EXIT FUNCTION  '<--- add  line in SQLitening.Bas and compile

Rather than test every program ever written, make above 1-line change to SQLitening.Bas
This is not an error in SQLitening, but a programming error not checking if database was open.
Calling slOpen will now just return if database is open and not create new handle and leak previous handle.

Test new SQLitening.dll to be sure duplicate handles are not created using slOpen

Code Select Expand

#INCLUDE "sqlitening.inc"  'test new sqlitening.dll with 3 open methods

SUB Test(sdatabase AS STRING)
 LOCAL h AS LONG
 slOpen sdatabase
 h = slgethandle
 slOpen sdatabase
 IF h <> slGetHandle THEN ? "Memory leak"
END SUB

FUNCTION PBMAIN () AS LONG
 Test "temp.tmp"  'test database file
 Test ":memory:"  'test in-memory database
 Test ""          'test temp database
 ? "Done"
END FUNCTION
Also tested over the internet and using threads.

[Fredrick Ughimi]

Hello CJ,

Thank you for the information.

Would this fix be included in the installation files?

Best regards.

[cj]

Fredrick,
I don't think it is a bug so I doubt it. That is up to Paul Squires.
Options:
1. Use slClose before slOpen.
2. Use IF ISFALSE(slIsOpen) THEN slOpen.
3. Don't call slOpen more than once without first closing.
4. Write a function or macro that replaces slOpen.
5  Add line IF thDab THEN EXIT FUNCTION like I did.

Fim,
Thank you for reporting this 4/1/2020 at:
https://sqlitening.planetsquires.com/index.php?topic=9756.msg26645;topicseen#msg26645

[cj]

Though I have not had a problem, I am removing this change in case
ATTACH or push the databases handles may need slOpen.  Better safe than sorry.

FUNCTION slOpen ALIAS "slOpen  ...
  IF thDab THEN EXIT FUNCTION  '<--- add  line in SQLitening.Bas and compile