• Welcome, Guest. Please login.
 
April 10, 2020, 01:12:57 am

News:

Welcome to the SQLitening support forums!


slDisconnect corrected

Started by cj, November 14, 2015, 10:08:15 pm

Previous topic - Next topic

cj

November 14, 2015, 10:08:15 pm Last Edit: November 19, 2015, 11:31:32 pm by cj
THIS IS NOT THE FINAL SOLUTION, PLEASE READ TO THE END
Disconnecting/connecting could cause the client program to  lock-up because CloseHandle thMutex was executed before the event was finished.  This mutex keeps the system-alive signal from mixing message with other messages.
This correction to SQLiteningClient.Bas should be compiled to your SQLiteningClient.DLL.
Frank W. Kelley pointed out the problem on November 11, 2015.
This should definitely be applied if using client/server mode.
Only 2-lines need to be remarked.  A test program demonstrating the problem is included at the bottom.

Correction to SQLiteningClient.Bas:
SUB SQLiteDisconnect ALIAS "SQLiteDisconnect" EXPORT
  ' Tell ImHere thread to die
  @tlpKillImHere = 1
'reset tlpKillImHere  ' Disconnect and close Tcp socket
  DoRequest %reqDisconnect, 0, 0, "", 0
  TCP CLOSE thSocket
'CloseHandle thMutex
END SUB




Test program, please change IP and Port:
$LocalIP = "192.168.1.2"
%PortNumber = 99999
%DisconnectWhenDone=1  ' 1 would lock-up before fix
%Threads = 10
%ConnectionsPerThread =3
#INCLUDE "win32api.inc"
#INCLUDE "sqlitening.inc"

GLOBAL gs AS STRING
'
FUNCTION PBMAIN AS LONG
  DIM hThread(1 TO %Threads) AS LONG, x AS LONG
  FOR x = 1 TO %Threads:THREAD CREATE MyThread(x) TO hThread(x):SLEEP 50:NEXT
  WaitforMultipleObjects UBOUND(hThread),hThread(1),%TRUE,%INFINITE
  FOR x = 1 TO %Threads:THREAD CLOSE hThread(x) TO hThread(x):NEXT
  IF %DisconnectWhenDone THEN
    ? gs,,"DisconnectWhenDone"
  ELSE
    ? gs,,"Disconnect not used"
  END IF
END FUNCTION
'
THREAD FUNCTION MyThread(BYVAL dummy AS LONG) AS LONG
  LOCAL x AS LONG, s() AS STRING
  LOCAL s AS STRING
  FOR x = 1 TO %ConnectionsPerThread
    slconnect $LocalIP,%PortNumber
    s+=USING$("connected thread # connection #",dummy,x) + $CR
    IF %DisconnectWhenDone THEN  'program locks up
      slDisconnect
    END IF
NEXT
UpdateMessage "Values in thread" + STR$(dummy) + $CR +  s
END FUNCTION

SUB UpdateMessage(s AS STRING) THREADSAFE
  gs+=s
END SUB

Bern Ertl

Thanks cj.  Nice work.

While the missing FreeLibrary calls apparently weren't causing this issue, I'd still recommend making the additions (as explained here). 

D. Wilson

First off I want to thank those involved in this forum and those who support SQLitening. I have had great success with SQLitening.

I have a couple of programs written in VB6 that utilize SQLitening. I use the SQLitening.Dll and  SQLiteningS.Dll.
Does this issue effect these Dll's. I have never experienced any issues before. it seems to be related to threads and I don't know if the Dll's that I use are involved with this issue.

Any Advice would be greatly appreciated.

Also I have an older copy of Power Basic. And honestly have not done much with them. Is it possible to get the fixes compiled in updated versions of SQLitening  If required.


cj

November 15, 2015, 10:06:13 pm #3 Last Edit: November 17, 2015, 02:39:12 pm by cj
SqliteningClient.Dll with
SUB SQLiteDisconnect ALIAS "SQLiteDisconnect" EXPORT
  @tlpKillImHere = 1 ' Tell ImHere thread to die
  'reset tlpKillImHere   '11/14/15 change 1
  DoRequest %reqDisconnect, 0, 0, "", 0
  TCP CLOSE thSocket   ' Disconnect and close Tcp socket
  'CloseHandle thMutex '11/14/15 change 2
END SUB     

Save or rename SQLiteningClient.JPG to  SQLiteningClient.DLL  (54,784 bytes)

D. Wilson

CJ thanks for the fast response. I will check it out. Like I said I have not experienced any problems with this issue.
I got it downloaded. Maybe we should get Paul to recompile the complete install package.

Bern Ertl

November 16, 2015, 12:22:07 pm #5 Last Edit: November 16, 2015, 12:27:12 pm by Bern Ertl
I'm examining cj's proposed fix for this problem and as I'm reviewing the SQLiteningClient.BAS code, I am wondering about a couple of things.

Best I can tell, this DLL is loaded for the first time with a call to SQLConnect.  It's the only function in the DLL where the mutex is created.  Looking at SQLConnect code, it calls DoRequest before creating the mutex (ie. thMutex is presumably 0 [still initialized]).  DoRequest immediately executes a call:

WaitForSingleObject thMutex, -1

So, the first time you connect, thMutex is zero, and I presume this WaitForSingleObject call fails (but returns so execution can continue).  The documentation for WaitForSingleObject doesn't really explain what it does if it passed an invalid handle (whether zero or otherwise).

With the unmodified SQLitening code (ie. not including cj's changes or my FreeLibrary additions), SQLiteningClient was closing the mutex upon slDisconnect, but keeping the handle (thMutex is never explicitly reset to zero), and not unloading (ie. the DLL was not unloaded).  So a subsequent call to SQLConnect would cause DoRequest to WaitForSingleObject on a non-zero, but invalid handle.  I presume that caused the lock up.

With cj's code and without my FreeLibrary addition, the initial mutex is never closed, SQLiteningClient is never unloaded (on slDisconnect calls) and subsequent slConnect calls will re-use the old mutex before creating a new mutex.  This will cause a small memory leak as every subsequent disconnect and connect will leave a mutex in the system that is never deleted/closed.

With both cj's AND my changes, everything should work as intended without a memory leak.

But, if I'm understanding this correctly, my suggested FreeLibrary additions (and without cj's code changes) should cause the DLL to unload upon slDisconnect.  Subsequent calls to slConnect should re-load the DLL and thMutex should be initialized, so I'm not clear on why that alone didn't also solve the lock up problem.

Bern Ertl

cj, with your proposed solution, run your test app.  When the msgbox appears on the screen, wait 21 seconds (don't click a button to close the msgbox).  I'm getting an exception thrown and the app crashes.  I presume the error is occurring when the 20 second SLEEP in ImHereThead ends and the function exits/ends.  I'm not sure why that is causing an error, but it's a pretty serious problem.

Also, instead of commenting out the two lines that you proposed, I tried adding a line to RESET thMutex after closing it.  cj's test app also works with this solution and I believe that it is a better solution as it is less likely to create memory leaks with unclosed mutexes (assuming we can fix the issue described above).

Correction to SQLiteningClient.Bas:
SUB SQLiteDisconnect ALIAS "SQLiteDisconnect" EXPORT
  ' Tell ImHere thread to die
  @tlpKillImHere = 1
  reset tlpKillImHere  ' Disconnect and close Tcp socket

  DoRequest %reqDisconnect, 0, 0, "", 0
  TCP CLOSE thSocket
  CloseHandle thMutex
  RESET thMutex
END SUB

This very simple test app also shows the problem described above.  The number of connections doesn't matter.  It's 20 seconds after the first disconnect that the app will crash.:
#COMPILE EXE
#DIM ALL
#INCLUDE ONCE "SQLitening.Inc"
$IP = "LocalHost"   '"192.168.1.2" 'change this
%PortNumber = 51234 'change this

%USEMACROS = 1
#INCLUDE ONCE "WIN32API.INC"
%IDD_CONNECTTEST =  101
%LISTBOX     = 1001

FUNCTION PBMAIN()
  LOCAL Result AS LONG
  LOCAL I AS LONG

  DO
    result = slConnect($IP,%PortNumber)
    IF result = 0 THEN
       slDisconnect
       INCR I
       IF MSGBOX( "Connected and disconnected " + FORMAT$( I) + " time(s). Try again?", %MB_YESNO) = %IDYES THEN ITERATE LOOP
       EXIT FUNCTION
    END IF
    BEEP
    SLEEP 1000
  LOOP UNTIL result <> 0
  MSGBOX "Failed to connect."

END FUNCTION

cj

Obviously the ImHere thread isn't finishing.  Back to the drawing board.


Bern Ertl

Not so obvious apparently.  I ran a test using my simple test code above and with an EXIT DO immediately after the SLEEP 20000 statement in ImHereThead (to explicitly force it to stop after the first 20 seconds).  I got the same exception/crash.

Something is going wonky when that thread function ends...

cj

If you find something let us know.
I have to do other things, but glad you reported it.
It is a logic thing that I'm definitely lacking on this one.
Goals:
CloseHandle thMutex in the correct place
Get ImHere thread to finish

I don't think I would want SQLite3.DLL or SQLiteningClient.DLL to unload until program ends.




Bern Ertl

Ah poop... Looks like the issue I'm seeing is related to my inclusion of the FreeLibrary calls in SQLitening.BAS (the change I was proposing).  I commented them out and now I'm not seeing the 20 second crash.  I'm guessing it's problematic to use FreeLibrary to unload a DLL when it has a thread function still running?

To summarize, do NOT add FreeLibrary calls as mentioned/referenced in my first post on this thread.  DO add the RESET thMutex statement to SQLiteDisconnect as mentioned previously.  Problems appear to be solved.

cj

Great, thank you!!
I  was spinning my wheels on CloseHandle thMutex

cj

November 17, 2015, 02:03:03 pm #12 Last Edit: November 17, 2015, 02:04:54 pm by cj
SUB SQLiteDisconnect ALIAS "SQLiteDisconnect" EXPORT
  @tlpKillImHere = 1
  RESET tlpKillImHere
  DoRequest %reqDisconnect, 0, 0, "", 0
  TCP CLOSE thSocket
  CloseHandle thMutex
  RESET thMutex
END SUB   

$LocalIP = "192.168.1.2"
%PortNumber = 12345
%Threads = 7
%ConnectionsPerThread =7
#INCLUDE "win32api.inc"
#INCLUDE "sqlitening.inc"
GLOBAL gs AS STRING
'
FUNCTION PBMAIN AS LONG
  DIM hThread(1 TO %Threads) AS LONG, x AS LONG
  FOR x = 1 TO %Threads:THREAD CREATE MyThread(x) TO hThread(x)
    SLEEP 50 'not the issue being discussed
  NEXT
  WaitforMultipleObjects UBOUND(hThread),hThread(1),%TRUE,%INFINITE
  FOR x = 1 TO %Threads:THREAD CLOSE hThread(x) TO hThread(x):NEXT
  ? gs,,"DisconnectWhenDone"
END FUNCTION
'
THREAD FUNCTION MyThread(BYVAL dummy AS LONG) AS LONG
  LOCAL x AS LONG, s() AS STRING
  LOCAL s AS STRING
  FOR x = 1 TO %ConnectionsPerThread
    slconnect $LocalIP,%PortNumber
    s+=USING$("connected thread # attempt #",dummy,x) + $CR
    slDisconnect
NEXT
UpdateMessage "Values in thread" + STR$(dummy) + $CR +  s
END FUNCTION
'
SUB UpdateMessage(s AS STRING) THREADSAFE
  gs+=s
END SUB




 

Frank W. Kelley

So, is this the final solution to this issue? (adding "RESET thMutex"?)


SUB SQLiteDisconnect ALIAS "SQLiteDisconnect" EXPORT
  @tlpKillImHere = 1
  RESET tlpKillImHere
  DoRequest %reqDisconnect, 0, 0, "", 0
  TCP CLOSE thSocket
  CloseHandle thMutex
  RESET thMutex
END SUB

cj

Found problem. 
Incorrectly fixed.
Corrected adding RESET thMutex as last line.
Started with this

SUB SQLiteDisconnect ALIAS "SQLiteDisconnect" EXPORT
   @tlpKillImHere = 1
   RESET tlpKillImHere
   DoRequest %reqDisconnect, 0, 0, "", 0
   TCP CLOSE thSocket
   CloseHandle thMutex
END SUB       

Changed to this

SUB SQLiteDisconnect ALIAS "SQLiteDisconnect" EXPORT
  @tlpKillImHere = 1
  DoRequest %reqDisconnect, 0, 0, "", 0
  TCP CLOSE thSocket
END SUB


Correct fix, add the line RESET thMutex to original code as the last line

SUB SQLiteDisconnect ALIAS "SQLiteDisconnect" EXPORT
  @tlpKillImHere = 1
  RESET tlpKillImHere
  DoRequest %reqDisconnect, 0, 0, "", 0
  TCP CLOSE thSocket
  CloseHandle thMutex
  RESET thMutex
END SUB