Leaks in SynSQLite3

Eric · 2013-01-04 10:14:52

I'm getting memory leaks with SynSQLite3 each time I open or close a DB, f.i. with just

procedure Test;
var
   db : TSQLDatabase;
begin
   db:=TSQLDatabase.Create(':memory:');
   db.DBOpen;
   db.DBClose;
   db.Free;
end;

The leaks come from sqlite3_memory_highwater through TSQLDatabase.DBOpen according to FastMM.

These are not one-time leaks, the more you call the above procedure, the more you leak. Any solutions?

ab · 2013-01-04 12:28:49

TSQLDatabase.Create already open the database.
This is clearly stated by the documentation:

    /// (re)open the database from file fFileName
    // - TSQLDatabase.Create already opens the database: this method is to be
    // used only on particular cases, e.g. to close temporary a DB file and
    // allow making a backup on its content
    function DBOpen: integer;
    /// close the opened database
    // - TSQLDatabase.Destroy already closes the database: this method is to be
    // used only on particular cases, e.g. to close temporary a DB file and
    // allow making a backup on its content
    procedure DBClose;

The leak comes from the fact that you call DBOpen twice.

Correct code is:

procedure Test;
var
   db : TSQLDatabase;
begin
   db:=TSQLDatabase.Create(':memory:');
   db.Free;
end;

I've added a check to raise an ESQLite3Exception if DBOpen method is called twice.
See http://synopse.info/fossil/info/f2a2b6da7c

Eric · 2013-01-04 12:56:01

Thanks for the extra check!

Eric · 2013-01-04 13:39:11

Btw, any chance of supporting the WideChar variants in SQLite? ie sqlite3_bind_text16, sqlite3_column_text16, etc. or are they missing from the .obj?

ab · 2013-01-04 14:10:12

We use indeed UTF-8 aspect only.
For the reasons exposed in the mORMot documentation.

But note that unless your database file is explicitly created with UTF-16 encoding, it will be slower to work with WideChar.

Thanks to the very optimized functions available in SynCommons.pas like UTF8ToString(), I suspect it won't make any difference.
With Unicode versions of Delphi, you can use RawUTF8 methods with the native UnicodeString type - the RTL will let Windows makes the conversion, but it is slower than the one implemented in SynCommons. So explicit calls of UTF8ToString/StringToUTF8 is preferred.

I've just added TSQLRequest.BindS() and TSQLRequest.FieldS() methods for direct VCL string process (in addition to mORMot native RawUTF8 type).
See http://synopse.info/fossil/info/371691b071

Eric · 2013-01-04 14:54:26

Nice! Thanks

While I have your ear, my I abuse by asking for a FieldDeclaredTypeS? (sqlite3_column_decltype...)

function TSQLRequest.FieldDeclaredTypeS(Col: Integer): String;
var P: PUTF8Char;
begin
  if cardinal(Col)>=cardinal(FieldCount) then
    raise ESQLite3Exception.Create(RequestDB, SQLITE_RANGE);
  P := pointer(sqlite3_column_decltype(Request,Col));
  result := UTF8DecodeToString(P,SynCommons.StrLen(P));
end;

ab · 2013-01-05 11:15:56

Done!
See http://synopse.info/fossil/info/6699709798603183a

I've taken a look at your new features of DWS.
Very nice!

For the HTTPS.SYS API version 2, I will certainly integrate it to Synopse, but perhaps with some syntax differences (e.g. using enumerations and sets for flags, instead of plain C constants).
By the way, how C# handle enumerations is just a PITA and a major break-down; pascal syntax just shines with them, and produces very readable, clean and fast code - what could be more powerful than an array[enumerates], as I (ab)use in mORMot source code? So powerful.

Note that some of your code already do not include fixes made for 64 bit compilation, I committed last days.
This is the limit of forks.

What I like particularly is how you handle SQL requests.
A TDataSet is a start, but we may benefit of something stronger, like an ORM.
What about your direct SQL statements in .dws files?

Using interfaces is very good, and will allow any future dev.

Eric · 2013-01-06 04:46:22

Thanks

ab wrote:

For the HTTPS.SYS API version 2, I will certainly integrate it to Synopse, but perhaps with some syntax differences (e.g. using enumerations and sets for flags, instead of plain C constants).

You might want to start from the xxm header translation instead (http://sourceforge.net/p/xxm/code/240/t … tpapi2.pas), it's more complete.
I only found it while I was almost done... and had hacked my way through the minimal header requirements (so I don't have everything in the http.sys 2.0)

ab wrote:

Note that some of your code already do not include fixes made for 64 bit compilation, I committed last days.
This is the limit of forks.

Well it was either fork or hack SynCrtSock to death because some methods and types were buried in the implementation section
And hacking to death would have been far more troublesome to keep sync'ed with your official version.

In the next release you'll split SynCrtSock in many smaller/simpler units so a new HttpAppi server can be subclassed more easily?

ab wrote:

A TDataSet is a start, but we may benefit of something stronger, like an ORM.

I went for the minimal interface that would be both functional and allow wrapping various db techs
Blob support is still MIA and some things are going to change (DataType will become an enum, a DeclaredType with the raw db type string, and probably some minimal standardized metadata queries)

I currently have a wrapper for SynSQlite, our proprietary drivers, and I would like to add some more open-source drivers wrappers, like for ZeosLib?
I've never used ZeosLib though, but I've spotted it in Synopse directories, and noticed some recent updates to it. This would be mostly to add support for the usual suspects (MySQL, SQLServer, Oracle, Frebird & PostgresQL).

ab wrote:

What about your direct SQL statements in .dws files?

Haven't started on it just yet, as I was wrapping up the server & setting up the minimal DB interfaces.
But this is certainly something I'll give a try, coupled with some JSON connector enhancements and beig able to expose script functions to SQLite, you could use it write things like

JSON.stringify(select field1, MyFunc(field2) from someTable where field3=@myVariable)

Which would be not just convenient, but also would catch SQL syntax errors at compile time.

ab · 2013-01-06 09:30:07

Eric wrote:

I currently have a wrapper for SynSQlite, our proprietary drivers, and I would like to add some more open-source drivers wrappers, like for ZeosLib?
I've never used ZeosLib though, but I've spotted it in Synopse directories, and noticed some recent updates to it. This would be mostly to add support for the usual suspects (MySQL, SQLServer, Oracle, Frebird & PostgresQL).

I started a Zeos fork, then it appeared to me that it was indeed big, unoptimized, and code very verbose / unmaintainable.
For instance, Oracle performance is awful, and you miss some nice features like array binding.
See http://blog.synopse.info/post/2012/07/1 … erformance - by the way, even DBExpress does not implement it. Only some third-party paid components do (http://www.devart.com/odac does provide it, and I do not like their Oracle 8-based direct access, whereas our units allow OCI libraries, which is much faster and maintained by Oracle).

This is why we added our own SynDB*.pas units, which are very efficient, and use very little code.
You can use them without mORMot.pas ORM/SOA units.
See the SAD pdf about it - http://synopse.info/files/pdf/Synopse%2 … 201.18.pdf (see "7. External database access" pages 126 and following) - and some articles in our blog, like http://blog.synopse.info/?q=syndb

If I were you, I would stay away from Zeos, or use DBExpress components.
The best solution could be some abstract interfaces, then several implementation classes targeting various DB providers.
I suspect that you will find out how SynDB*.pas unit are easy to work with.

Eric · 2013-01-06 14:00:56

ab wrote:

If I were you, I would stay away from Zeos, or use DBExpress components.
The best solution could be some abstract interfaces, then several implementation classes targeting various DB providers.
I suspect that you will find out how SynDB*.pas unit are easy to work with.

Okay, good to know, I'll go directly for SynDB then!

We've been evolving our own DB connection layers over the years @work, so I haven't used any of the bundled Delphi DB components in years (except to test occasionally... and shake head in disbelief). Having a new source of fast & tested drivers is good!

Also you mentioned ORM, I've not looked into how you're tying mORMot to the servers yet, f.i. to have a ".dws" handler cooperate with a native Delphi mORMot handler.
I guess you're supposed to subclass, check the request, and then call inherited if the request is not handled? (if so, there would be nothing special required for me to make the dws http server compatible with other Delphi-based mORMot handlers)

Eric · 2013-01-07 08:34:19

Btw, just added SynDBODBC wrapper, worked like a charm

ab · 2013-01-07 08:44:52

I suspect FireBird native driver (without OleDB nor ODBC layer) will be the next one in my list.
There is already a NexusDB around (developed by some mORMot users), but not yet integrated to the main trunk.

Which DB are you using?

Eric · 2013-01-07 09:42:01

Firebird usually, Oracle & MS SQL for larger uses or when the customer already has licenses/admins, and SQLite for everything else.

For Firebird we used to use IBX a long time ago, but due to various bugs and bloat, we've fallen down to the FreeIB layer that IBX was built upon, with a few fixes, it's still running smoothly. There doesn't seem to be any official place for FreeIB anymore, or I would have published the fixes (even though these aren't extensive, and are essentially related to adding Unicode support and fixing minor blob issues), though I'm not sure what the Borland license headers mean (and so if those files are really redistributable from the Borland version, if the original open-source version should be used, or if headers should be remade from scratch from the C headers).

ab · 2013-01-07 10:10:28

Could you send us your FreeIB layer? webcontact01 at synopse dot info
For the headers, I probably will include them in the implementation section of SynDBFireBird unit, as for the other layers.

Why did you specify SynDBODBC for DWS?
I suppose an abstract layer for SynDB would make better sense.
SynDB classes are pretty abstract, and from the same exact classes, you can create and access directly to all kind of providers: Oracle, ODBC, OleDB, and even SQlite3 which has its own optimized class (so it is better IMHO than a SynSQLiteDatabase unit).

If you need some other methods to SynDB classes, feel free to ask.

Eric · 2013-01-07 15:47:49

ab wrote:

Could you send us your FreeIB layer? webcontact01 at synopse dot info

Sent.

ab wrote:

Why did you specify SynDBODBC for DWS?

because that's the only unit I looked at initially
I've re-based on SynDB, I'll commit so you can take a look.

I'm getting a leak in simple statements, apparently TSQLDBStatementWithParamsAndColumns has a fColumn field of TDynArrayHashed that isn't freed, and that's the only leak I'm seeing... the FastMM4 log says

This block was allocated by thread 0x544, and the stack trace (return addresses) at the time was:
40495D [System.pas][System][@ReallocMem][3935]
409932 [System.pas][System][@DynArraySetLength][26579]
87A8C7 [SynCommons.pas][SynCommons][TDynArrayHashed.AddAndMakeUniqueName][19969]
8F0ED6 [SynDBODBC.pas][SynDBODBC][TODBCStatement.BindColumns][1082]
8F2393 [SynDBODBC.pas][SynDBODBC][TODBCStatement.Prepare][1441]
8F3B4A [dwsSynDBDatabase.pas][dwsSynDBDatabase][TdwsSynDBDataSet.Create][227]
8F3ABA [dwsSynDBDatabase.pas][dwsSynDBDatabase][TdwsSynDBDataBase.Query][211]

ab · 2013-01-07 19:51:38

TDynArrayHashed is not a class, but an object.
There is no getmem nor nested class created...
Sounds like a compiler issue - just as http://blog.synopse.info/post/2011/01/2 … elphi-2010
I just hate such basic regressions at compiler level!

But I was not able to find out a similar issue in this case.
Running SynDB + SynDBSQLite3 is perfectly without memory leak from my tests with FastMM4...
I'll check with SynDBODBC.

Eric · 2013-01-08 07:41:27

ab wrote:

But I was not able to find out a similar issue in this case.

It was with SynDBODBC.

Compiler bug confirmed: issue comes and goes when building/rebuilding... and that's an "object" as in the other bug you linked.

Though since it uses inheritance and virtual methods, it can't be replaced by a record, but has to become a class, which means various other changes in the rest of the code to create/destroy manually, but that's probably the only option as compiler support for "object" is becoming weaker and weaker.

ab · 2013-01-08 08:00:53

It does not use virtual methods, just inheritance.

OK - I'll make the code refactoring.

The more EMB publishes updates of their compiler, the more I want to switch to FPC.
Do they have regression tests? Sounds like if they ignore the huge code base available on the customer side.

ab · 2013-01-08 13:17:56

Huge refactoring to circumvent the compiler bug...
See http://synopse.info/fossil/info/7ff2f00300

Still a record, so can be allocated on the stack.
Just some inlined methods to access to the nested TDynArray record.

Bojop · 2013-01-08 14:07:07

Hi Ab,

ab wrote:

Huge refactoring to circumvent the compiler bug...
See http://synopse.info/fossil/info/7ff2f00300
Still a record, so can be allocated on the stack.
Just some inlined methods to access to the nested TDynArray record.

after this i got these errors and i use XE

[DCC Error] SynCommons.pas(2617): E2184 PROTECTED section valid only in class types
[DCC Error] SynCommons.pas(3014): E2184 PROTECTED section valid only in class types
[DCC Fatal Error] SynSSPIAuth.pas(61): F2063 Could not compile used unit 'SynCommons.pas'

Eric · 2013-01-08 15:23:32

Works all right AFAICT once the 'protected' are replaced by 'private' at the lines mentioned by Bojop

ab · 2013-01-08 16:23:38

Indeed.
Fixed by http://synopse.info/fossil/info/d6fb701d51 and http://synopse.info/fossil/info/f049f54313

I did not have any Unicode version of Delphi at hand, when I made the fix.

ab · 2013-01-09 08:30:28

I just checked out your latest updates.
Nice seeing SynDB classes reused as such.

You did use UTF8Encode() function in your wrappers, whereas I suggest to use StringToUTF8() from SynCommons.pas, which is much faster.

I've just added FireBird support via ODBC, and will certainly publish a direct FireBird access unit soon.
Note also that I've added some specific connection properties for MS SQL 2005, 2008 or 2012 (in addition to the generic MS SQL driver, which was in fact SQL 2008/2012).

Eric · 2013-01-09 09:49:04

Updated & simplified the wrappers.

I've added UIB driver, it's a bit heavier in terms of code size and dependencies, but full-featured and shows it's simple to wrap just any db layer.

chapa · 2013-01-09 11:34:10

Do not know if here is the place to report, but know Eric will see it.

Eric, in order to compile the server (Delphi XE3) I have to correct following:

according MSDN,
at DSimpleDWScript.pas: procedure TSimpleDWScript.SetCPUAffinity(), variables procMask, systemMask used in calling GetProcessAffinityMask() should be PDWORD_PTR (in Delphi DWORD_PTR=>ULONG_PTR=>NativeUInt)
at dwsDirectoryNotifier.pas: procedure TdwsFileNotifier.Execute, variable completionKey used in calling GetQueuedCompletionStatus() should be PULONG_PTR (ULONG_PTR=>NativeUInt)

Great work!

Eric · 2013-01-09 11:55:00

chapa wrote:

Do not know if here is the place to report, but know Eric will see it.

You can post in the issue tracker (http://code.google.com/p/dwscript/issues/list)

chapa wrote:

Eric, in order to compile the server (Delphi XE3) I have to correct following

This will require some ifdef'ing: in XE and previous versions, all those functions are declared as "var xxx : cardinal".

As I don't have XE3 to test, what should those ifdefs be?f.i. does this work in XE3?

   {$if CompilerVersion < 24.0} // XE3
   completionKey : DWORD;
   {$else}
   completionKey : DWORD_PTR;
   {$ifend}

chapa · 2013-01-09 12:29:58

Yes, works.

dwsDirectoryNotifier.pas:

   {$if CompilerVersion < 24.0} // XE3
   completionKey : DWORD;
   {$else}
   completionKey : ULONG_PTR;
   {$ifend}

DSimpleDWScript.pas

   {$if CompilerVersion < 24.0} // XE3
   procMask, systemMask : Cardinal;
   {$else}
   procMask, systemMask : DWORD_PTR;
   {$ifend}

Also, in XE3 many TCriticalSection, TObjectList, etc. methods are declared as inline for good, and relevant units should be declared in uses list in order compiler to inline the calls.
This is related to both DWS and mORMot. Annoying thousands compiler hints occur.

I can provide support for XE3.

Do you plan to add WebSockets support on HttpSys2 server side?

Eric · 2013-01-09 13:34:49

chapa wrote:

I can provide support for XE3.

XE3 uses could be problematic due to the dotted namespaces, though as long as the aliases work, that should be okay (ie. not looking forwards to ifdef's in the uses clauses...)

chapa wrote:

Do you plan to add WebSockets support on HttpSys2 server side?

Eventually yes, though the relevant functionality was added in Win8/Win2012, and I have access to neither at the moment...

ab · 2013-01-09 15:00:07

I suppose that for WebSockets support, we may better use a plain "socket" server, not http.sys.

In mORMot, I would like to use it, but also provide an alternate "long polling" implementation, which is much more router/IT friendly.

chapa · 2013-01-09 16:11:14

ab, why you think "Socket" server will be more suitable? compatibility?

ab · 2013-01-09 16:17:52

We already spoke about that.
It is due to http.sys support of WebSocket to be limited to Win8/Win2012 and not Win7/Win2008, and that AFAIK there is no direct API to handle it.
It is implemented in the .Net framework layer over http.sys, not directly in its own http.sys API.

In all cases, websockets is a trolling subject.
Do not speak of them with your IT people.
HTTP is stateless, and websockets just break the specification and expectation of the protocol.
"Everything over HTTP" is IMHO just another monstrosity.

WebSockets are widely used to develop clients over the Internet with the same "interactivity" than over a local network, just like good old desktop apps.
But is is also a way of adding network traffic, breaking the proxy/cache/firewall/WAS policies, and the open door to any kind of DOS attacks.

For mORMot, we may enjoy websockets for the upcoming events collaboration features, but we won't implement it only with it, but also with class "long polling" connections, and other named pipe or GDI messages.
Our RESTful scheme is perfectly stateless, so websockets are not mandatory at all for building a whole client-server application using mORMot.

Eric · 2013-01-10 07:39:47

For classic client-server I don't think WebSockets are really useful, as the browser will maintain connections (especially over SSL), and WebSockets frames have packaging as well that main not be wholly more efficient (unless you have large cookies...).

However WebSockets works well for pushing content and notifications, where it will have less server overhead than long polling, and less latency than periodic polling.

What I have in mind for websockets would basically be to attach them to a "named group", each named group would be linked to a processing (a script f.i.) and a set of in-memory properties. Any other server-side processing could post messages/content to a "named group", which would then be dispatched to all clients on the other side of the websockets.

This should AFAICT solve the need for push notifications very simply: just attach all the websockets to the same "named group" / notification channel.
And individual (stateful) websocket connections would be named groups with only one websocket in them.

ab · 2013-01-10 09:35:43

This is what mORMot will implement with Events publish/subscribe mechanism.
See http://blog.synopse.info/post/2012/09/0 … laboration

But I do not understand when you write "individual (stateful) websocket connections would be named groups with only one websocket in them"...
Do you mean one websocket per client, which will receive all the corresponding notification callbacks?
And one websocket per client, and per named group?

Eric · 2013-01-10 10:19:02

By stateful I mean when you want to use the websocket not to push notifications, but to establish a connection to the server in which the actions related to a frame sent through the websocket aren't fully defined by that frame, but also defined by previous frames, and where that state isn't shared with other websockets (like in FTP or SMTP f.i.).

That makes a it a special case where the group consists of a single websocket (and then you only have to generate a unique name for the group, using Time+Random or whatever)

The concept of "client" wouldn't be part of the mechanism, it would be something you would add on top, and same goes for multiplexing on the same websocket.

mORMot Open Source

#1 2013-01-04 10:14:52

Leaks in SynSQLite3

#2 2013-01-04 12:28:49

Re: Leaks in SynSQLite3

#3 2013-01-04 12:56:01

Re: Leaks in SynSQLite3

#4 2013-01-04 13:39:11

Re: Leaks in SynSQLite3

#5 2013-01-04 14:10:12

Re: Leaks in SynSQLite3

#6 2013-01-04 14:54:26

Re: Leaks in SynSQLite3

#7 2013-01-05 11:15:56

Re: Leaks in SynSQLite3

#8 2013-01-06 04:46:22

Re: Leaks in SynSQLite3

#9 2013-01-06 09:30:07

Re: Leaks in SynSQLite3

#10 2013-01-06 14:00:56

Re: Leaks in SynSQLite3

#11 2013-01-07 08:34:19

Re: Leaks in SynSQLite3

#12 2013-01-07 08:44:52

Re: Leaks in SynSQLite3

#13 2013-01-07 09:42:01

Re: Leaks in SynSQLite3

#14 2013-01-07 10:10:28

Re: Leaks in SynSQLite3

#15 2013-01-07 15:47:49

Re: Leaks in SynSQLite3

#16 2013-01-07 19:51:38

Re: Leaks in SynSQLite3

#17 2013-01-08 07:41:27

Re: Leaks in SynSQLite3

#18 2013-01-08 08:00:53

Re: Leaks in SynSQLite3

#19 2013-01-08 13:17:56

Re: Leaks in SynSQLite3

#20 2013-01-08 14:07:07

Re: Leaks in SynSQLite3

#21 2013-01-08 15:23:32

Re: Leaks in SynSQLite3

#22 2013-01-08 16:23:38

Re: Leaks in SynSQLite3

#23 2013-01-09 08:30:28

Re: Leaks in SynSQLite3

#24 2013-01-09 09:49:04

Re: Leaks in SynSQLite3

#25 2013-01-09 11:34:10

Re: Leaks in SynSQLite3

#26 2013-01-09 11:55:00

Re: Leaks in SynSQLite3

#27 2013-01-09 12:29:58

Re: Leaks in SynSQLite3

#28 2013-01-09 13:34:49

Re: Leaks in SynSQLite3

#29 2013-01-09 15:00:07

Re: Leaks in SynSQLite3

#30 2013-01-09 16:11:14

Re: Leaks in SynSQLite3

#31 2013-01-09 16:17:52

Re: Leaks in SynSQLite3

#32 2013-01-10 07:39:47

Re: Leaks in SynSQLite3

#33 2013-01-10 09:35:43

Re: Leaks in SynSQLite3

#34 2013-01-10 10:19:02

Re: Leaks in SynSQLite3

Board footer