Dalija Prasnikar

5 minutes ago, limelect said:

@Dalija Prasnikar see in one of my answers I even

free all used components DB and link nothing helped.

Once the program reaches max memory nothing helps

But if I use 2 programs just for testing see above it works.

Even on the same DB

Then the problem is not in this particular code but all the other code that consumes all available memory. The general advice for reducing memory footprint I gave earlier still stand.

Yes, using separate applications will help because each will have its own memory limit if they are running on 64-bit OS.

30 minutes ago, limelect said:

"Don't use memory stream to load data to rich edit. Create temporary file and write the data into that file and then load the data from the filestream."

I tried all ready  weeks ago

The question is whether or not you have done that correctly. The moment you write the data to file, you need to release memory of all the temporary data that was used in that process before you try to load that into control.

  MyMemStream :=  ProjectsFDTable.CreateBlobStream(ProjectsFDTable.FieldByName('Description'),bmRead);
  try
    TMemoryStream(MyMemStream).SaveToFile('t.txt'); // using absolute path is preferred
  finally
    MyMemStream.Free;
  end;
  Memo1.BeginUpdate;
  try
    Memo1.LoadFromFile('t.txt');
  finally
    Memo1.EndUpdate;
  end;

On 12/9/2022 at 6:03 PM, limelect said:

MyMemStream:=  ProjectsFDTable.CreateBlobStream(ProjectsFDTable.FieldByName('Description'),bmRead);
 AdvRichEditor.BeginUpdate;
AdvRichEditor.LoadFromStream(MyMemStream);
AdvRichEditor.EndUpdate;

Don't use memory stream to load data to rich edit. Create temporary file and write the data into that file and then load the data from the filestream.

However, no matter what you do, at some point you will run into memory issues with 32-bit application if your application consumes a lot of memory. I suggest you switch to 64-bit. If you cannot switch to 64-bit, then the only other option is optimizing other parts of your application to reduce overall memory usage. 

First step for optimizing would be getting rid of all memory leaks - they can cause fragmentation which can be a huge problem.

Next, reducing total memory allocation, by releasing anything that is not used at the time.  Be careful with creating local components that have owner which you don't explicitly free. They will not show as memory leaks, but every time you call such method your memory consumption will increase.

If that does not help, then you can try reducing memory load by avoiding default string type which is Unicode since D2009 (you haven't said which Delphi version you use) and use UTF-8 encoded strings - UTF8String and convert to Unicode only when you need to display data to the user - this is really the last option as you may need to write a lot of supporting code to avoid accidental conversions to and from Unicode strings as they will slow your application down and use extra memory in the process.

What is main? 

You are most likely corrupting a memory by accessing same data from main thread and background thread. Also you shouldn't work with any visual control from the background thread without calling the TThread.Synchronize or TThread.Queue

Also you are working with pointers, so it is possible that you are accidentally overwriting something.

4 hours ago, alogrep said:

I found where the problem occurs (see apptached image). However the call stack doesn't go inside the procedure where the problem actually happes. This is the code

There is a thread involved. Most likely not all data that needs to be protected is protected and another thread is either releasing the object while it is still in use, or there is memory corruption because multiple threads are messing with the data.

But there is not enough code to pinpoint the problem. 

1 hour ago, Anders Melander said:

I haven't QP'd it and will probably not have time to do so anytime soon; We've just updated our main product suite to Delphi 11.2 (from 10.3) and there's plenty of other stuff that needs my attention.

I just looked at that code and the problem is not in the inline variable. Casting as interface creates hidden interface reference no matter what and that reference hinders release. This behavior is not a regression and it is not related to inline variables so I wouldn't hope it will be fixed soon.

Reported as https://quality.embarcadero.com/browse/RSP-40166 

5 hours ago, Anders Melander said:

Casting a function result with the interface as operator and the old behavior is back. Reproduced with Delphi 11.2.

I suppose you tested those outside the main code block. Have you filed QP report?

11 minutes ago, microtronx said:

Since we're all crazy programmers ... is "No" acceptable? Any explanation of why No?

It is complicated...

In theory you can have Windows application that has multiple GUI threads with each one holding own windows message loop.

The answer is no, because VCL is build on the premise of single main (GUI) thread (like many other frameworks as there is really no advantage of having such multithreaded GUI) and you would have to write your own framework to achieve such application and probably would have to rewrite significant parts of the RTL.

No.

25 minutes ago, John R. said:

Nice! I'm currently using events from Spring4D and at first sight, this looks similar. Or am I missing something ?

I have no idea how similar it is to Spring Events as I never used them. 

9 hours ago, juergen said:

So I would also be very happy to see a simple example.

Here is simple example:

Declare event in some independent unit C

type
  TMyEvent = type string;

Unit A

procedure TFormA.ButtonClick(Sender: TObject);
begin
  NxHorizon.Instance.Post<TMyEvent>('New Caption');
end;

 
Unit B

type
  TFormB = class(...
    procedure FormCreate(Sender: TObject);
    procedure FormDestroy(Sender: TObject);
  protected 
    fMyEventSubcription: INxEventSubscription;
    procedure OnMyEvent(const aEvent: TMyEvent);    
  end;
    
procedure TFormB.FormCreate(Sender: TObject);
begin
  fMyEventSubcription: := NxHorizon.Instance.Subscribe<TMyEvent>(MainAsync, OnMyEvent);
end;

procedure TFormB.FormDestroy(Sender: TObject);
begin
  if Assigned(fMyEventSubscription) then NxHorizon.Instance.Unsubscribe(fMyEventSubscription);
end;

procedure TFormB.OnMyEvent(const aEvent: TMyEvent);
begin
  Label.Caption := aEvent;
end;

Since you are working with GUI, events should be dispatched either as MainAsync or MainSync and that will ensure they run in the context of the main thread. Also, because they run on the main thread, you can just unsubscribe and you don't need to WaitFor subscription to finish work as there will be no active work done in the background threads.

I have put subscribing and unsubscribing in FormCreate and FormDestroy event handlers, but you can also put them anywhere else where you are doing form initialization or finalization. Or, you can subscribe and unsubscribe at some other point.

6 hours ago, Jeff Steinkamp said:

I took a page out of this Indy demo and applied it to my code, and once I got the FileStream creating straightened out for the thread, it is functioning quite well indeed!.  I have attached the file for that unit if you want to have a look and see if there is anything that can be done differently.

It just needs few tweaks.

First, you don't need Application.ProcessMessages anywhere - they have been commented out, but the whole point of threads is getting rid of those. So you can just forget about it 🙂

Next, this will work fine, but in case user closes the dialog before download completes already queued synchronization methods will access released form and crash. To prevent that, you need to add additional filed in Form declaration and OnCloseQuery event handler on the form. I also fixed try finally block inside thread as constructing HTTP instance can also fail (not likely event, but still), and in that case the cleanup was not proper one. I also added try..except block to show error message in case of failure. 

If the whole application is closed then anonymous threads can be killed at any time. If this is a possibility, then you might want to replace TThread.CreateAnyonymousThread with TTask.Run (without Start) from System.Threading as all running tasks will be waited for on application shutdown. 

FileDownload.pas

FileDownload.dfm

You can find simple demo how to use threads with Indy here: https://github.com/dalijap/code-delphi-thread-safety/tree/master/Part3/19 Indy

This demo is using anonymous thread, but you can also use regular threads or TTask from Parallel Programming library, too. 

If you can post your code I can give you more detailed advice.

1 hour ago, Fr0sT.Brutal said:

Probably he meant that in case of many parameters they are pushed to stack not to registers. Stack is somewhat related to memory 🙂

Uh.... this is such far fetched speed optimization that it is not even worth thinking about.

And if you start measuring particular solutions, you may easily find that you will not get very far with them.

1 hour ago, Stano said:

I am a layman. I remember that the number of arguments 3 is related to memory management.

Memory management has nothing to do with number of parameters in constructors.

There are other reasons why having too many parameters is considered "bad", but those are all in line that having too many parameters can be indication that your class is doing too much - violates Single Responsibility Principle.

In real life, having classes and functions that require a lot of parameters and that are not in violation of SRP, nor can be adequately split, are quite common, and often workarounds for solving such "issues" are more convoluted than having few parameters over the top.

If there is really a need to have class fully initialized during construction and too many parameters poses an issue that cannot be meaningfully solved by splitting the class, then Builder pattern can be suitable solution. There is usually a lot of repetitive code involved in writing a Builder class, so it is useful only if you need to construct particular class in plenty of places across your application.

The main problem with many parameters, especially when they are of same type, is that you can easily pass the wrong data to wrong parameter. This could be solved on IDE level, by showing names of the parameters as hints within code - not visible outside the IDE,, so calling the constructor or function would look like 

TPerson.Create(AFirstName: 'John', ALastName: 'Doe');

Another similar feature on language level are named parameters, where you can or must explicitly write parameter name in the code, similar to the above.

5 minutes ago, Ian Branch said:

Where does that go and why?

Implementation section where your TTracksAddEditForm class is declared. Just like with any other method that is declared in a class, its implementation must be included inside that unit implementation section. 

21 minutes ago, Ian Branch said:

.  Using a constructor.  Never knowingly done so.  I tried Dalija's constructor but it gives me an E@065 Unsatisfied forward or external declaration: 'TTracksAddEditForm.Create'.  Not an issue, I don't believe I will go that route.

Oops... I forgot to copy-paste that part of the code... 

constructor TTracksAddEditForm.Create(AOwner: TComponent; const AsMode: string; AiAlbum: integer; const AsTrack: string);
begin
  inherited Create(AOwner);
  FsMode := AsMode;
  FiAlbum := AiAlbum;
  FsTrack := AsTrack;
end;

1 hour ago, Ian Branch said:

I guess I should ask, is there a better way to do this?

Your code can be simplified in several ways. I will write each step separately, so you can more easily adapt your code if you need additional functionality beyond the code shown here.

First, as I mentioned in previous post, you don't need to write getter and setter functions at all, if you just read or write field in those and noting more. First optimization would be removing all that redundant code.  This is not just speed optimization, but also makes cleaner code that is easier to read and follow. This code is functionally completely equivalent to your declaration and you would create and initialize the form the same way you do now.

  TTracksAddEditForm = class(TForm)
  private
    FsMode: string;
    FiAlbum: integer;
    FsTrack: string;
  public
    property sMode: string read FsMode write FsMode;
    property iAlbum: integer read FiAlbum write FiAlbum;
    property sTrack: string read FsTrack write FsTrack;
  end;

Your next question was whether you need to have both getters and setters. If you are just setting property, then you don't need getter. Similarly, if you have read only property that you will just read, then you don't need setter.

So next optimization would be following. There is no difference in how you create and initialize your form.

  TTracksAddEditForm = class(TForm)
  private
    FsMode: string;
    FiAlbum: integer;
    FsTrack: string;
  public
    property sMode: string write FsMode;
    property iAlbum: integer write FiAlbum;
    property sTrack: string write FsTrack;
  end;

procedure TAudioForm.btnInsertTrackClick(Sender: TObject);
begin
  TracksAddEditForm := TTracksAddEditForm.Create(Self);
  TracksAddEditForm.sMode := 'Insert';
  TracksAddEditForm.iAlbum := Albums.FieldByName('Index').AsInteger;
  TracksAddEditForm.sTrack := TracksView.FieldByName('Title').AsString;
  TracksAddEditForm.Show;
end;

But, the best way to initialize any class that you will construct through code (without bringing in dependency injection) would be declaring all required information as parameters to the constructor, Such code would also require different code when constructing form. 

  TTracksAddEditForm = class(TForm)
  private
    FsMode: string;
    FiAlbum: integer;
    FsTrack: string;
  public
    constructor Create(AOwner: TComponent; const AsMode: string; AiAlbum: integer; const AsTrack: string); reintroduce;
  end;

procedure TAudioForm.btnInsertTrackClick(Sender: TObject);
begin
  TracksAddEditForm := TTracksAddEditForm.Create(Self, 'Insert', Albums.FieldByName('Index').AsInteger, TracksView.FieldByName('Title').AsString);
  TracksAddEditForm.Show;
end;

The advantage of passing all required data as parameters during construction process is that you cannot accidentally forget to initialize some required field. If some fields are optional, then you can stick to initializing through properties, but using simplified examples before this last one.

59 minutes ago, Ian Branch said:

Can some one point me to a plain English explanation of what Getters & Setters do and how they do it.

Getters and Setters are methods called when you get (read) or set (write) property. In Delphi you don't need to have getter and setter methods. Properties can be directly connected to a field. 

You use getters or setters when you want to run additional code besides directly reading or writing some field value. 

Following is basic property declaration without any getters or setters. When you access property, compiler just directly reads or writes from associated field.

  TFoo = class
  protected
    FValue: Integer;
  published
    property Value: Integer read FValue write FValue;
  end;

Following is property declaration with setter method (you can have any combination you need and you don't need to declare both methods if you just need one of them)

  TFoo = class
  protected
    FValue: Integer;
    procedure SetValue(AValue: Integer);
  published
    property Value: Integer read FValue write SetValue;
  end;

Because getters and setters are methods, they will be a bit slower than directly using a field. If your setter is just setting a field and does nothing more, then you don't really need it. Same goes for getter. If it just returns the field then it is not needed.

procedure TFoo.SetValue(AValue: Integer);
begin
  FValue := AValue;
end;

If for instance you don't want to allow setting Value to zero, you might have following code in a setter.

procedure TFoo.SetValue(AValue: Integer);
begin
  if AValue <> 0 then
    FValue := AValue;
end;

You can even raise an exception if you set property to invalid value or similar.

The only time when you would need to use getters and setters that just read or write to a field, is when your property needs to be accessible through interface. Interfaces require that getters and setters are methods. 

14 minutes ago, Uwe Raabe said:

Under Tools - Configure Tools add a new entry named Kill LSP with the following settings:

 

Code:

Program: taskkill
Parameters: /IM DelphiLSP.exe /F

Can you please post that as Q/A on Stack Overflow. It will be more easily found there.

Loading resource strings should be thread-safe. Issue probably happens because you are loading string resource to a shared string variable. Without seeing your code it is impossible to tell exactly.

11 hours ago, Ian Branch said:

Leaving aside the use of 'FreeAndNil()' and 'with' discussions, to me this is a simpler structure, easier to code and read, and it works.  Those things make my life easier.. 😉

I don't think you can avoid that 

'with' has many pitfalls and replacing code that does not use 'with' with code using 'with' is not something I would recommend doing. 

11 hours ago, Ian Branch said:

So To my question - As you can see in the first code I use FreeAndNil(), as much for peace of mind, imagined or real, as anything else.

Is there a way to implement the equivalent in the second code structure?

You have to ask yourself what is the purpose of FreeAndNil? In the case of local variable the reason (regardless whether we consider it valid or not) is to prevent accidental access of the object instance after it has been released. In such cases we hope that accessing nil reference would crash immediately revealing the obvious bug in the code.

However, when you use 'with' you no longer have explicit reference to the object that you can accidentally access outside the 'with' block. Yes, if you are really sloppy, you can still call Free first and that call some other code if call to Free is not the last thing you do in the 'with' block. Main point is compiler hides the reference, there is nothing you can nil here, and compiler will certainly not access the instance outside the 'with' block by mistake.

Calling Destroy in destructor without having nil check first is definitely incorrect code that can lead to memory leaks as destructor needs to be able to handle partially constructed instances. That is why Free is commonly used as it does not require sprinkling destructors with nil checks before calling Destroy. 

But, very likely this bad code in the destructor is not the primary cause of the issues you are seeing and that there is more going on. If the destructor is at fault, then this means that the instance was not properly constructed and that construction raised an exception. Otherwise the core problem is in some other code we don't see here.

20 hours ago, DavidOne said:

What do you think about this solution?

Is it really thread safe and may be a good workaround?

This is appropriate solution.

Code that runs inside those anonymous methods will run in the context of the main thread (this is defined by first boolean parameter passed to ExecuteAsync - if you pass False there then that code will run in the context of the background thread). 

Because it runs in the context of the main thread it will be thread-safe the same way the same way your original function is thread-safe if you run it from the main thread.

The only thing you need to pay attention to, is that RESTRequest, RESTClient, and RESTResponse used for making asynchronous request can only handle single request at the time in thread-safe manner. That means, you cannot run UserRESTRequest.ExecuteAsync again while the previous request is not yet completed. Once the request completes, you can reuse those REST components to make another request. 

There are two separate concerns here.

First, main application code block can exhibit some weird behavior for global variables declared there, including inline variables. If you want to make proper test of some behavior, I suggest wrapping the code in additional procedure. 

Second, inline variables had (and maybe still have) some problems and sometimes using them does not work as expected. If you encounter an issue, usually solution is to declare local variable instead of inline.

Your text case does not work correctly in 10.3.3 when code runs in main code block. If moved to the procedure, then it behaves as it should. In 10.4.2 and and 11.2 it works correctly in all scenarios, so the issue was fixed in the meantime.