sempare / sempare-delphi-template-engine

Sempare Template (scripting) Engine for Delphi allows for flexible dynamic text generation. It can be used for generating email, html, reports, source code, xml, configuration, etc.
Apache License 2.0
144 stars 18 forks source link

Concurrency issue #205

Closed craigmox closed 1 month ago

craigmox commented 1 month ago

I've found that usage of a class instance of TRegEx in Sempare.Template.Lexer.pas is not thread-safe and is causing memory corruption issues in my testing:

https://github.com/sempare/sempare-delphi-template-engine/blob/2bbdf4c158d5cfe6b2d03cc50991cff703368918/src/Sempare.Template.Lexer.pas#L272

  function IsValidId(const AId: string): Boolean;
  begin
    exit(FIDRegex.IsMatch(AId));
  end;

This is due to the implementation of IsMatch unfortunately using a field to store the input text:

function TRegEx.IsMatch(const Input: string): Boolean;
begin
  FRegEx.Subject := Input;
  Result := FRegEx.Match;
end;

I've found that System.SysUtils.IsValidIdent is a suitable replacement and has been working in my testing, just as one suggestion.

Thanks!

darnocian commented 1 month ago

Ah. Good spot and great suggestion.

darnocian commented 1 month ago

fix is on dev branch and will be pushed next week

darnocian commented 1 month ago

Just wanted to check - where you testing with Template.Eval() methods, or via the template registry methods?

I'll add some more multithreaded tests.

craigmox commented 1 month ago

I've been load testing one of our web services that funnels through the class function Template.Parse(const AContext: ITemplateContext; const AStream: TStream; const AManagedStream: boolean = true): ITemplate overload.

darnocian commented 1 month ago

Possibly the reason I didn't notice that the way I normally do things in load templates upfront, and only call Template.Eval() on the template with data when it comes to the the services handling requests. The Template Registry has more synchronisation elements in it to also support loading. Just mentioning in case you were not aware of it.

BTW if you could review a small questionaire, it would be appreciated: https://docs.google.com/forms/d/e/1FAIpQLScioIiDxvsWK01fMFqYr9aJ6KhCGeiw4UaU_esGuztEE7vYwA/viewform

It would be great if you could give a small testimonial. Thanks.