Gary 17 Posted December 29, 2023 Hello all, I am trying to separate my UI from the rest of my application. I want to use an interfaced listener on my main form and pass a reference to it around to parts of the application that need input from the user. I have no problem with the InputBox or MsgDialog as they don't take too many parameters, however I want to use TMSTaskDialog as well (Different listener since requires 3rd party). The problem I am having is that the TaskDialog has so many options that I want to pass a record with all the parameters. I only have a problem with TStrings. I get memory leak on close. Work in progress but here are the relevant parts. TTaskDialogParams = record DisplayedButtons : TPsTaskDialogDisplayedButtons; Title: String; Instruction: string; Content: string; Options: TPsTaskDialogOptions; CustomButtons : TStringList; Icon: TPsTaskDialogIcon; FResultCallBack: TProc<TTaskDialogReturnParams>; procedure Clear; class operator Initialize (out Dest: TTaskDialogParams); class operator Finalize (var Dest: TTaskDialogParams); end; IPsFNCTaskDialogUIListener = interface ['{BA6ABA84-F3EE-4421-856F-42E76C878D3B}'] procedure ExecuteDialog(const Params : TTaskDialogParams); end; procedure TTaskDialogParams.Clear; begin CustomButtons.Clear; CustomButtons.Free; end; class operator TTaskDialogParams.Finalize(var Dest: TTaskDialogParams); begin // Dest.CustomButtons.Free; end; class operator TTaskDialogParams.Initialize(out Dest: TTaskDialogParams); begin Dest.CustomButtons := TStringList.Create; end; In listener I set string params by enumerating through the recordsStringList. procedure TPsFNCTaskDialogUIListener.ExecuteDialog(const Params : TTaskDialogParams); begin SetDialogParams(Params); FTaskDialog.Execute; Params.Clear; // FTaskDialog.CustomButtons.Clear; end; Then show it procedure TPsFNCTaskDialogUIListener.ExecuteDialog(const Params : TTaskDialogParams); begin SetDialogParams(Params); FTaskDialog.Execute; Params.Clear; // FTaskDialog.CustomButtons.Clear; end; Some code in there is things I have tried to no avail. Any help would be appreciated. I tried this before and gave up and used an Interfaced Class instead. Share this post Link to post
Gary 17 Posted December 29, 2023 Sorry copied same block twice Here is the other part procedure TPsFNCTaskDialogUIListener.SetDialogParams(Params: TTaskDialogParams); begin ClearDialog; FTaskDialog.Title := Params.Title; FTaskDialog.Instruction := Params.Instruction; FTaskDialog.Icon := TTMSFNCTaskDialogIcon(Params.Icon); FTaskDialog.Options := TTMSFNCTaskDialogOptions(params.Options); for var s : String in params.CustomButtons do FTaskDialog.CustomButtons.Add(s); end; Share this post Link to post
Brian Evans 100 Posted December 29, 2023 "Leak" is a bit vague - usually there would be some indication of what leaked which can help narrow down what to look at. Share this post Link to post
Gary 17 Posted December 29, 2023 From FastMM --------------------------------2023/12/28 22:32:03-------------------------------- A memory block has been leaked. The size is: 84 This block was allocated by thread 0x6A4, and the stack trace (return addresses) at the time was: 9371E6 [System.pas][System][@GetMem$qqri][4960] 938957 [System.pas][System][TObject.NewInstance][18324] 93905A [System.pas][System][@ClassCreate$qqrpvzc][19654] 9F9C30 [System.Classes.pas][System.Classes][Classes.TStringList.Create][8121] 94C231 [FastMM4.pas][FastMM4][UpdateHeaderAndFooterCheckSums$qqrp29Fastmm4.TFullDebugBlockHeader][9153] 94D214 [FastMM4.pas][FastMM4][DebugGetMem$qqri][9731] EA116F [int.PsFNCTaskDialogUIListener.pas][Int.PsFNCTaskDialogUIListener][Psfnctaskdialoguilistener.TTaskDialogParams._op_Initialize][66] 108E6E5 [PsFNCTaskDialogUIListener.pas][PsFNCTaskDialogUIListener][TPsFNCTaskDialogUIListener.SetDialogParams][32] 108E827 [PsFNCTaskDialogUIListener.pas][PsFNCTaskDialogUIListener][TPsFNCTaskDialogUIListener.ExecuteDialog][47] 108F3C9 [USender.pas][USender][TfrmSender.tbtnTaskDialogClick][77] C0CF34 [FMX.Controls.pas][FMX.Controls][Controls.TControl.GetHeight][5512] The block is currently used for an object of class: System.Classes.TStringList The allocation number is: 296325 Current memory dump of 256 bytes starting at pointer address 7E6DB770: 20 B9 9C 00 00 00 00 00 50 B9 30 7F 6C E3 1B 7F 00 00 00 00 00 00 00 00 2C 00 22 00 3D 00 0E 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 16 8C AF 78 80 80 80 80 80 80 80 80 00 00 00 00 51 BB 6D 7E 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 88 85 04 00 9F A8 93 00 3C AD 93 00 A2 CC 93 00 F2 E6 08 01 27 E8 08 01 C9 F3 08 01 34 CF C0 00 9D AB C0 00 77 D6 CB 00 F4 B2 C0 00 D4 65 E8 00 A4 06 00 00 A4 06 00 00 02 72 93 00 59 A9 93 00 28 C5 93 00 E2 D3 C0 00 39 8B 93 00 6E 89 93 00 A5 90 93 00 2E 9A CB 00 B3 57 E5 00 ED 92 93 00 45 C0 E8 00 48 00 00 00 B0 04 02 00 CB 3D A4 8E 8C 7C 0C 01 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 . . . . . P 0 l . . . . . . . . . , . " . = . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . x . . . . Q m ~ . . . . . . . . . . . . . . . . . . . < . . . . ' . . . . 4 . . w . . e . . . . . . . . r . Y . ( . . 9 . n . . . . W . . E . H . . . . . . = | . . --------------------------------2023/12/28 22:32:03-------------------------------- A memory block has been leaked. The size is: 20 This block was allocated by thread 0x6A4, and the stack trace (return addresses) at the time was: 9371E6 [System.pas][System][@GetMem$qqri][4960] 93A89F [System.pas][System][@NewUnicodeString$qqri][26024] 93AD3C [System.pas][System][@UStrAsg$qqrr20System.UnicodeStringx20System.UnicodeString][26946] 9F6F30 [System.Classes.pas][System.Classes][Classes.TStrings.Create][6729] 9F9C3D [System.Classes.pas][System.Classes][Classes.TStringList.Create][8122] 94D214 [FastMM4.pas][FastMM4][DebugGetMem$qqri][9731] EA116F [int.PsFNCTaskDialogUIListener.pas][Int.PsFNCTaskDialogUIListener][Psfnctaskdialoguilistener.TTaskDialogParams._op_Initialize][66] 108E6E5 [PsFNCTaskDialogUIListener.pas][PsFNCTaskDialogUIListener][TPsFNCTaskDialogUIListener.SetDialogParams][32] 108E827 [PsFNCTaskDialogUIListener.pas][PsFNCTaskDialogUIListener][TPsFNCTaskDialogUIListener.ExecuteDialog][47] 108F3C9 [USender.pas][USender][TfrmSender.tbtnTaskDialogClick][77] C0CF34 [FMX.Controls.pas][FMX.Controls][Controls.TControl.GetHeight][5512] The block is currently used for an object of class: UnicodeString The allocation number is: 296326 Current memory dump of 256 bytes starting at pointer address 7F1BE360: B0 04 02 00 01 00 00 00 02 00 00 00 0D 00 0A 00 00 00 AA 4B E7 F7 80 80 00 00 00 00 A1 FF 1B 7F 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 5F BF 04 00 E6 71 93 00 57 89 93 00 5A 90 93 00 7F 21 E8 00 12 EE E8 00 31 8A E8 00 1F 40 E8 00 63 40 E8 00 6E EE E8 00 62 DF A0 00 6B 8A 93 00 A4 06 00 00 A4 06 00 00 02 72 93 00 75 89 93 00 A5 90 93 00 E5 21 E8 00 6B 8A 93 00 35 38 E8 00 02 72 93 00 75 89 93 00 A5 90 93 00 F7 10 A8 00 6B 8A 93 00 14 00 00 00 DC E2 E6 00 4C 29 7E 8F 8C 7C 0C 01 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 80 B3 D6 81 70 00 00 00 00 C1 E9 1B 7F 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 9F BF 04 00 E6 71 93 00 57 89 93 00 5A 90 93 00 73 35 E3 00 1B 35 E3 00 51 C4 E2 00 13 C5 E2 00 3E 3F E8 00 D1 F1 E8 00 0A 3A E8 00 02 72 93 00 . . . . . . . . . . . . . . . . . K . . . . . . . . . . . . . . . . . . . . . _ . . q . W . Z . ! . . . 1 . . @ . c @ . n . b . k . . . . . . . . r . u . . ! . k . 5 8 . . r . u . . . . k . . . . . . L ) ~ | . . p . . . . . . . . . . . . . . . . . . . . . . . q . W . Z . s 5 . . 5 . Q . . . > ? . . . : . . r . --------------------------------2023/12/28 22:32:03-------------------------------- This application has leaked memory. The small block leaks are (excluding expected leaks registered by pointer): 13 - 20 bytes: UnicodeString x 1 69 - 84 bytes: System.Classes.TStringList x 1 Note: Memory leak detail is logged to a text file in the same folder as this application. To disable this memory leak check, undefine "EnableMemoryLeakReporting". Share this post Link to post
Kas Ob. 109 Posted December 29, 2023 2 hours ago, Gary said: A memory block has been leaked. The size is: 84 This block was allocated by thread 0x6A4, and the stack trace (return addresses) at the time was: 9371E6 [System.pas][System][@GetMem$qqri][4960] 938957 [System.pas][System][TObject.NewInstance][18324] 93905A [System.pas][System][@ClassCreate$qqrpvzc][19654] 9F9C30 [System.Classes.pas][System.Classes][Classes.TStringList.Create][8121] It is from the above that this is one time leak, which is caused by not freeing the stringlist class operator TTaskDialogParams.Finalize(var Dest: TTaskDialogParams); begin // Dest.CustomButtons.Free; end; So where is the problem ? What happen if you did uncomment that line ? Share this post Link to post
mvanrijnen 121 Posted December 29, 2023 1 hour ago, Kas Ob. said: It is from the above that this is one time leak, which is caused by not freeing the stringlist class operator TTaskDialogParams.Finalize(var Dest: TTaskDialogParams); begin // Dest.CustomButtons.Free; end; So where is the problem ? What happen if you did uncomment that line ? then he get's an AV, cause he's freeing the CustomButtons in his Clear method. Share this post Link to post
Dalija Prasnikar 1382 Posted December 29, 2023 It is very likely that you would have less issues with interface based class and then you would be able to pass around interface reference without having to worry about memory management too much. If you still want to use record, you need to fix your Clear procedure. You are calling CustomButtons.Free there which will destroy CustomButtons instance and then you would have crash in Finalize which would try to destroy that same instance. You should just clear the list and not free it there. So that memory management happens solely within Initialize and Finalize methods. procedure TTaskDialogParams.Clear; begin CustomButtons.Clear; end; class operator TTaskDialogParams.Finalize(var Dest: TTaskDialogParams); begin Dest.CustomButtons.Free; end; 2 Share this post Link to post
Kas Ob. 109 Posted December 29, 2023 40 minutes ago, mvanrijnen said: then he get's an AV, cause he's freeing the CustomButtons in his Clear method. He was asking about the memory leak and i pointed where the leak came form, an AV is his wrong design/approach. Share this post Link to post
Remy Lebeau 1330 Posted December 30, 2023 (edited) Also, because SetDialogParams() is taking a TTaskDialogParams by value, a copy is made, which will have its own TStringList object, so you must implement the Assign() operator as well to copy the source record's TStringList data to the copied record after it is Initialized: class operator TTaskDialogParams.Assign(var Dest: TTaskDialogParams; const [ref] Src: TTaskDialogParams); begin Dest.CustomButtons.Assign(Src.CustomButtons); end; class operator TTaskDialogParams.Finalize(var Dest: TTaskDialogParams); begin Dest.CustomButtons.Free; end; class operator TTaskDialogParams.Initialize(out Dest: TTaskDialogParams); begin Dest.CustomButtons := TStringList.Create; end; Otherwise, if you don't do this, the default assignment behavior will copy the object pointer instead, causing the copied record to lose its own TStringList object (leaking it) and end up pointing to the original TStringList object, so when the copied record is Finalized then the original TStringList object gets freed, corrupting the original record that was copied from. That being said, there is no good reason to pass the record by value in the code shown. SetDialogParams() should take the record by const reference instead, then no copy is made. Read the documentation, especially the section on "Passing Managed Records as Parameters" : https://docwiki.embarcadero.com/RADStudio/en/Custom_Managed_Records Edited December 30, 2023 by Remy Lebeau 5 1 Share this post Link to post