DavidJr. 1 Posted January 18, 2022 (edited) Hi, I have an application that takes CAD model data and slices it until layers then posts these layers as binary data using Indy and IdMultipartForm (AddFile()). I am usually not allowed to ask for help outside the company because of the proprietary information, but I will do my best at giving as much information as I can. Most of the time this just works but occasionally I get the following: System.SysGetMem(Opt.out) System._GetMem(???) System._NewUnicodeString(1) System._UStrFromPWCharLen('','sA',1) System._UStrFromWChar(???,'s') IdGlobalProtocols.FindFirstNotOf(#9' ','slice',5,1) IdCoderHeader.DecodeHeader('slice') IdCoderHeader.EncodeHeader('slice','','B','UTF-8') IdMultipartFormData.TIdFormDataField.FormatHeader IdMultipartFormData.TIdFormDataField.GetFieldSize IdMultipartFormData.TIdMultiPartFormDataStream.CalculateSize IdMultipartFormData.TIdMultiPartFormDataStream.IdSeek(???,???) IdGlobal.TIdBaseStream.Seek(???,???) System.Classes.TStream.GetSize The application receives the data from a queue, processes it with 100% success, and then the file is written locally to a a disk, then I do the following: // EDITED: took out "BAD SECTIONS" in attempt to get a better response. function TMain.SaveTheSolvedSlice(FileName: String): Boolean; var MimeType, Response: String; jsonVal: TJSonValue; Stat, Size: String; SendParams: TIdMultiPartFormDataStream; TOPLSliceFile: TStream; //HTTPSendFile: TIdHTTP; begin Result := False; Response := 'NULL'; HTTPSendFile.Request.Clear; HTTPSendFile.ReadTimeout := 2000; HTTPSendFile.ConnectTimeout := 9000; SendParams := TIdMultiPartFormDataStream.Create; Application.ProcessMessages; try if(FileExists('tmp\out\'+FileName)) then begin MimeType := 'application/octet-stream'; While(GetFileSizeInBytes('tmp\out\'+FileName) < MinSliceSizeFromUnsolvedFile+1) Do Delay(10); //Params.AddFormField('_token', 'Myfb9OqYgDBwDws3zTL9QOs492XWfNtGLftUdNsH'); SendParams.AddFile('slice', 'tmp\out\'+FileName, MimeType).ContentTransfer:='binary'; Sleep(10); SendParams.Position := 0; try While(GetFileSizeInBytes('tmp\out\'+FileName) > SendParams.Size) Do Delay(10); finally end; StatusUpdate(Console, 'Attempting to post file '+'tmp\out\'+FileName+' (size: '+SendParams.Size.ToString+' bytes)'); Delay(10); Response := HTTPSendFile.Post(STORAGE_OUT_REPO, SendParams); Delay(5); if(Response <> 'NULL') then begin //{"Slice":{"Status":"ACCEPTED","FileName":"1_Some_Job_Filename.slc","Size":1812}} jsonVal := TJSonObject.ParseJSONValue(Response); Stat := jsonVal.GetValue<String>('Slice.Status'); Size := jsonVal.GetValue<String>('Slice.Size'); jsonVal.Free; StatusUpdate(Console, 'Toasted Slice (size: '+Size+') '+Stat.ToLower); if(Stat = 'ACCEPTED') then Result := True else if(Stat = 'ERROR') then Result := False else Result := False; end; end finally SendParams.Free; Response := 'NULL'; HTTPSendFile.Response.Clear; end; end; I do the comparison of size of original file to that of the Params, which I expect to be slightly larger, to ensure the file has finished writing to disk, then loaded as a File in TIdMultiPartFormDataStream before attempting to post back to API. I suspect that I am trying to get the the size of the Params object too soon, is that likely? Also I use the Indy IdHTTP as object that I dragged into the form, which works better than declaring a new instance and freeing it every time I process a new file. Also note: the method GetFileSizeInBytes() is a method I created a few years back based on example code and always works. Edited January 18, 2022 by DavidJr. Share this post Link to post
Fr0sT.Brutal 900 Posted January 18, 2022 Hmm, it's rare to see SO many bad practices in one piece of code... 1 Share this post Link to post
DavidJr. 1 Posted January 18, 2022 This isn't used anywhere other than for testing and I am new to Indy/Delphi. But your comment didn't tell me anything that was helpful. If you are referring to the excessive try blocks or "Delay()" Please give me any suggestions. The Delay is really a replacement for sleep used here. procedure Delay(TickTime: Integer); var Past, Present: LongInt; begin Past := GetTickCount; repeat if MsgWaitForMultipleObjects(0, Pointer(nil)^, FALSE, (TickTime - Present), QS_ALLINPUT) <> WAIT_OBJECT_0 then Break; Application.ProcessMessages; Present := (GetTickCount - Past); until ( Present >= LongInt(TickTime) ); end; I normally do not need call a "delay" but here it it seemed to allow for the file to be loaded into a TStream as a Multipart form file field. So this was just attempt at that. I have not used Indy IdHTTP before this and I have no issues writing to files using TFileStream, but it seemed like in this case I needed to wait before Posting the file. Share this post Link to post
DavidJr. 1 Posted January 18, 2022 More information. The filename that's passed into the function is ALWAYS present so the while loop that checks for file existence doesn't need to be there, but for testing purposes I added it. Share this post Link to post
Fr0sT.Brutal 900 Posted January 18, 2022 (edited) Application.ProcessMessages looks quite nice to push it everywhere thus achieving responsible main thread while doing things but it's highway to hell. F.ex., you're not insured from 2nd TMain.SaveTheProcessed run while 1st one still in action. Then, you don't need most of these Delay at all as operations the procedure performs are synchronous. And you have too messy proc - divide it to "wait for file" subproc and "send file" subproc, remove waitings and then test... Edited January 18, 2022 by Fr0sT.Brutal Share this post Link to post
DavidJr. 1 Posted January 18, 2022 Actually, I only procedure after calling SaveTheProcessed if the return is True... so the app will not continue on a false. I edited the function above in my IDE and in this forum... I will take out those "Delays". But can you clarify on the statement regarding "wait for file"? Thank you for the tips you offered so far. I do appreciate the help. Share this post Link to post
Remy Lebeau 1392 Posted January 18, 2022 (edited) 2 hours ago, DavidJr. said: Most of the time this just works but occasionally I get the following Indicating what exactly? A memory leak? Quote I do the comparison of size of original file to that of the Params, which I expect to be slightly larger, to ensure the file has finished writing to disk I would not do it that way. TIdMultipartFormDataStream.AddFile() opens the file for read-only access and denies further writing to the file. If you can't wait for the file to be fully written to disk before calling SaveTheSolvedSlice(), then I would suggest using a TFileStream in a loop to open the file for exclusive access until successful, and then pass the opened TFileStream to TIdMultipartFormDataStream.AddFormField(TStream), eg: function TMain.SaveTheSolvedSlice(FileName: String): Boolean; var MimeType, Response, FullFilePath: String; jsonVal: TJSonValue; Stat, Size: String; SendParams: TIdMultiPartFormDataStream; TOPLSliceFile: TStream; //HTTPSendFile: TIdHTTP; begin Result := False; FullFilePath := 'tmp\out\'+FileName; if (not FileExists(FullFilePath)) then Exit; Response := 'NULL'; HTTPSendFile.Request.Clear; HTTPSendFile.ReadTimeout := 2000; HTTPSendFile.ConnectTimeout := 9000; SendParams := TIdMultiPartFormDataStream.Create; try MimeType := 'application/octet-stream'; repeat try TOPLSliceFile := TFileStream.Create(FullFilePath, fmOpenRead or fmExclusive); except on E: EFOpenError do begin // I know, this is not the best option, but it is the only way to detect // a sharing violation error with TFileStream. The alternative is to call // CreateFile/FileOpen() and GetLastError() directly, and then create a // THandleStream from the result... if E.Message <> 'The process cannot access the file because it is being used by another process.' then raise; Delay(100); end; end; until False; try //Params.AddFormField('_token', 'Myfb9OqYgDBwDws3zTL9QOs492XWfNtGLftUdNsH'); SendParams.AddFormField('slice', MimeType, '', TOPLSliceFile, FileName).ContentTransfer := 'binary'; Delay(10); StatusUpdate(Console, 'Attempting to post file ' + FullFilePath+' (size: ' + TOPLSliceFile.Size.ToString + ' bytes)'); Delay(10); Response := HTTPSendFile.Post(STORAGE_OUT_REPO, SendParams); Delay(5); if (Response <> 'NULL') then begin //{"Slice":{"Status":"ACCEPTED","FileName":"1_Some_Job_Filename.slc","Size":1812}} jsonVal := TJSonObject.ParseJSONValue(Response); try Stat := jsonVal.GetValue<String>('Slice.Status'); Size := jsonVal.GetValue<String>('Slice.Size'); finally jsonVal.Free; end; StatusUpdate(Console, 'Toasted Slice (size: ' + Size + ') ' + Stat.ToLower); Result := (Stat = 'ACCEPTED'); end; finally TOPLSliceFile.Free; end; finally SendParams.Free; end; end; Edited January 18, 2022 by Remy Lebeau Share this post Link to post
DavidJr. 1 Posted January 18, 2022 2 minutes ago, Remy Lebeau said: Indicating what exactly? A memory leak? I would not do it that way. TIdMultipartFormDataStream.AddFile() opens the file for read-only access and denies further writing to the file. If you can't wait for the file to be fully written to disk before calling SaveTheSolvedSlice(), then I would suggest using a TFileStream in a loop to open the file for exclusive access until successful, and then pass the opened TFileStream to TIdMultipartFormDataStream.AddFormField(TStream). I am not sure if it is a memory leak. Nothing else is running when I call SaveTheSolvedSlice() ... I actually do use TFileStream to write the file to disk first. That always works without a single issue. So is TFileStream blocking? What I mean by that is does it complete before moving on? function ExportSliceToBinFile(MyFileName: String): Boolean; var I: Integer; MySliceFile: TFileStream; begin if(Not DirectoryExists('tmp')) then CreateDir('tmp'); if(Not DirectoryExists('tmp\out')) then CreateDir('tmp\out'); MySliceFile := TFileStream.Create('tmp\out\'+MyFileName, fmCreate or fmOpenWrite); try FOR I := 1 TO GlobalIndex DO begin MySliceFile.WriteBuffer(GlobalSliceRecord[I], SizeOf(SLICE_RECORD)); end; Result := True; except Result := False; end; MyOPLToolPathFile.Free; end; The code above never fails... Share this post Link to post
DavidJr. 1 Posted January 18, 2022 To Clarify the "memory leak" answer, I always Free any object after use, unless it is created design time like the IdHTTP instance. Share this post Link to post
Remy Lebeau 1392 Posted January 18, 2022 (edited) 22 minutes ago, DavidJr. said: I am not sure if it is a memory leak. Then why mention it? Quote So is TFileStream blocking? Yes, it is. Quote What I mean by that is does it complete before moving on? Yes. Though, behind the scenes, the OS may take some time to actually flush the data to the physical disk in the background, but its data cache will handle any requests for file data that hasn't been flushed yet. So, in that regard, if you KNOW the saving process has finished writing its data to the file before you call SaveTheSolvedSlice(), then you don't need the loop to wait to access the file. Just open the file and it will either success or fail, act accordingly: function TMain.SaveTheSolvedSlice(FileName: String): Boolean; var MimeType, Response, FullFilePath: String; jsonVal: TJSonValue; Stat, Size: String; SendParams: TIdMultiPartFormDataStream; Param: TIdFormDataField; //HTTPSendFile: TIdHTTP; begin Result := False; FullFilePath := 'tmp\out\'+FileName; if (not FileExists(FullFilePath)) then Exit; Response := 'NULL'; HTTPSendFile.Request.Clear; HTTPSendFile.ReadTimeout := 2000; HTTPSendFile.ConnectTimeout := 9000; SendParams := TIdMultiPartFormDataStream.Create; try MimeType := 'application/octet-stream'; //Params.AddFormField('_token', 'Myfb9OqYgDBwDws3zTL9QOs492XWfNtGLftUdNsH'); Param := SendParams.AddFile('slice', FullFilePath, MimeType); Param.ContentTransfer := 'binary'; StatusUpdate(Console, 'Attempting to post file ' + FullFilePath + ' (size: ' + Param.FieldStream.Size.ToString + ' bytes)'); Delay(10); Response := HTTPSendFile.Post(STORAGE_OUT_REPO, SendParams); Delay(5); if (Response <> 'NULL') then begin //{"Slice":{"Status":"ACCEPTED","FileName":"1_Some_Job_Filename.slc","Size":1812}} jsonVal := TJSonObject.ParseJSONValue(Response); try Stat := jsonVal.GetValue<String>('Slice.Status'); Size := jsonVal.GetValue<String>('Slice.Size'); finally jsonVal.Free; end; StatusUpdate(Console, 'Toasted Slice (size: ' + Size + ') ' + Stat.ToLower); Result := (Stat = 'ACCEPTED'); end; finally SendParams.Free; end; end; That being said, your ExportSliceToBinFile() is leaking MySliceFile, so it is leaving the file open. MyOPLToolPathFile.Free() should be MySliceFile.Free() instead. And it should be protected with a try..finally: function ExportSliceToBinFile(MyFileName: String): Boolean; var I: Integer; MySliceFile: TFileStream; begin try ForceDirectories('tmp\out'); MySliceFile := TFileStream.Create('tmp\out\'+MyFileName, fmCreate or fmOpenWrite); try for I := 1 to GlobalIndex do begin MySliceFile.WriteBuffer(GlobalSliceRecord[I], SizeOf(SLICE_RECORD)); end; finally MySliceFile.Free; end; Result := True; except Result := False; end; end; Edited January 18, 2022 by Remy Lebeau Share this post Link to post
DavidJr. 1 Posted January 18, 2022 2 minutes ago, Remy Lebeau said: Then why mention it? Yes, it is. Yes. Though, behind the scenes, the OS may take some time to actually flush the data to the physical disk in the background, but its data cache will handle any requests for file data that hasn't been flushed yet. So, in that regard, if you KNOW the saving process has finished writing its data to the file before you call SaveTheSolvedSlice(), then you don't need the loop to wait to access the file. Just open the file and it will either success or fail, act accordingly. That being said, your ExportSliceToBinFile() is leaking MySliceFile. MyOPLToolPathFile.Free() should be MySliceFile.Free() instead. And it should be protected with a try..finally: function ExportSliceToBinFile(MyFileName: String): Boolean; var I: Integer; MySliceFile: TFileStream; begin try ForceDirectories('tmp\out'); MySliceFile := TFileStream.Create('tmp\out\'+MyFileName, fmCreate or fmOpenWrite); try for I := 1 to GlobalIndex do begin MySliceFile.WriteBuffer(GlobalSliceRecord[I], SizeOf(SLICE_RECORD)); end; finally MySliceFile.Free; end; Result := True; except Result := False; end; end; Yes.. I caught that, Actually I had renamed the other references from MyOPL... to MySliceFile so I did not have to explain any meaning behind the name but as you posted is how it really is, sorry about that. The logic involved it creating the data has been in place for years and is very consistent but also contains proprietary info. Share this post Link to post
aehimself 396 Posted January 18, 2022 2 hours ago, DavidJr. said: repeat if MsgWaitForMultipleObjects(0, Pointer(nil)^, FALSE, (TickTime - Present), QS_ALLINPUT) <> WAIT_OBJECT_0 then Break; Application.ProcessMessages; Present := (GetTickCount - Past); until ( Present >= LongInt(TickTime) ); I wanted to save time before to convert blocking methods to background threads but still blocking the calling method while it runs... used a cycle like this. Once issues started to appear I spent months getting rid of this "shortcut". It became way more messy and way more complicated. Even if you don't have issues at the moment... remember, this cycle is the devil reincarnated. 1 Share this post Link to post
DavidJr. 1 Posted January 19, 2022 21 hours ago, Remy Lebeau said: Indicating what exactly? A memory leak? I would not do it that way. TIdMultipartFormDataStream.AddFile() opens the file for read-only access and denies further writing to the file. If you can't wait for the file to be fully written to disk before calling SaveTheSolvedSlice(), then I would suggest using a TFileStream in a loop to open the file for exclusive access until successful, and then pass the opened TFileStream to TIdMultipartFormDataStream.AddFormField(TStream), eg: function TMain.SaveTheSolvedSlice(FileName: String): Boolean; var MimeType, Response, FullFilePath: String; jsonVal: TJSonValue; Stat, Size: String; SendParams: TIdMultiPartFormDataStream; TOPLSliceFile: TStream; //HTTPSendFile: TIdHTTP; begin Result := False; FullFilePath := 'tmp\out\'+FileName; if (not FileExists(FullFilePath)) then Exit; Response := 'NULL'; HTTPSendFile.Request.Clear; HTTPSendFile.ReadTimeout := 2000; HTTPSendFile.ConnectTimeout := 9000; SendParams := TIdMultiPartFormDataStream.Create; try MimeType := 'application/octet-stream'; repeat try TOPLSliceFile := TFileStream.Create(FullFilePath, fmOpenRead or fmExclusive); except on E: EFOpenError do begin // I know, this is not the best option, but it is the only way to detect // a sharing violation error with TFileStream. The alternative is to call // CreateFile/FileOpen() and GetLastError() directly, and then create a // THandleStream from the result... if E.Message <> 'The process cannot access the file because it is being used by another process.' then raise; Delay(100); end; end; until False; try //Params.AddFormField('_token', 'Myfb9OqYgDBwDws3zTL9QOs492XWfNtGLftUdNsH'); SendParams.AddFormField('slice', MimeType, '', TOPLSliceFile, FileName).ContentTransfer := 'binary'; Delay(10); StatusUpdate(Console, 'Attempting to post file ' + FullFilePath+' (size: ' + TOPLSliceFile.Size.ToString + ' bytes)'); Delay(10); Response := HTTPSendFile.Post(STORAGE_OUT_REPO, SendParams); Delay(5); if (Response <> 'NULL') then begin //{"Slice":{"Status":"ACCEPTED","FileName":"1_Some_Job_Filename.slc","Size":1812}} jsonVal := TJSonObject.ParseJSONValue(Response); try Stat := jsonVal.GetValue<String>('Slice.Status'); Size := jsonVal.GetValue<String>('Slice.Size'); finally jsonVal.Free; end; StatusUpdate(Console, 'Toasted Slice (size: ' + Size + ') ' + Stat.ToLower); Result := (Stat = 'ACCEPTED'); end; finally TOPLSliceFile.Free; end; finally SendParams.Free; end; end; Thank you for the help. This shows that the file is still being used by another process. How do I fix that? This is where the file is created: function ExportSliceToBinFile(MyFileName: String): Boolean; var I: Integer; {// OPL_ToolPath_File_Type: ###DRMJ###} MyOPLToolPathFile: TFileStream; // of OPL_COMMAND_RECORD; begin if(Not DirectoryExists('tmp')) then CreateDir('tmp'); if(Not DirectoryExists('tmp\out')) then CreateDir('tmp\out'); MyOPLToolPathFile := TFileStream.Create('tmp\out\'+MyFileName, fmCreate or fmOpenWrite); try try FOR I := 1 TO GlobalToolpathIndex DO begin MyOPLToolPathFile.WriteBuffer(NewOPLToolPath[I], SizeOf(OPL_COMMAND_RECORD)); end; Result := True; except Result := False; end; finally MyOPLToolPathFile.Free; end; end; The thread that calls this is where the processing is also done. How can I make sure the file is complete released after creation? Share this post Link to post
DavidJr. 1 Posted January 19, 2022 22 hours ago, Remy Lebeau said: repeat try TOPLSliceFile := TFileStream.Create(FullFilePath, fmOpenRead or fmExclusive); except on E: EFOpenError do begin // I know, this is not the best option, but it is the only way to detect // a sharing violation error with TFileStream. The alternative is to call // CreateFile/FileOpen() and GetLastError() directly, and then create a // THandleStream from the result... if E.Message <> 'The process cannot access the file because it is being used by another process.' then raise; Delay(100); end; end; until False; actually this of course would result in an exception if I am repeating the same thing.. But I think my method for getting file size was not freeing in time. because when I stropped calling it everything was fine. Thanks again for the help. Share this post Link to post
Remy Lebeau 1392 Posted January 19, 2022 (edited) 4 hours ago, DavidJr. said: Thank you for the help. This shows that the file is still being used by another process. How do I fix that? First, you need to figure out where the file is actually open. Use a tool like SysInternals Process Explorer to see who has open handles to the file. If it turns out to be your app, then check your code to make sure you are closing all of your open handles to the file, and that if you need to open multiple handles then don't open them with conflicting permissions. 2 hours ago, DavidJr. said: But I think my method for getting file size was not freeing in time. because when I stropped calling it everything was fine. You do know it is possible to get a file's size without actually opening a handle to the file, don't you? You can get the file's size from the filesystem's metadata for the file, such as with SysUtils.FindFirst(), rather than querying the size from the file itself. You shouldn't be using the file size as an indicator that the file is ready for use, though... Edited January 19, 2022 by Remy Lebeau Share this post Link to post
DavidJr. 1 Posted January 19, 2022 (edited) 47 minutes ago, Remy Lebeau said: First, you need to figure out where the file is actually open. Use a tool like SysInternals Process Explorer to see who has open handles to the file. If it turns out to be your app, then check your code to make sure you are closing all of your open handles to the file, and that if you need to open multiple handles then don't open them with conflicting permissions. You do know it is possible to get a file's size without actually opening a handle to the file, don't you? You can get the file's size from the filesystem's metadata for the file, such as with SysUtils.FindFirst(), rather than querying the size from the file itself. You shouldn't be using the file size as an indicator that the file is ready for use, though... I was using this: function GetFileSizeInBytes(const fn: string): integer; var sr : TSearchRec; begin if FindFirst(fn, faAnyFile, sr ) = 0 then result := Int64(sr.FindData.nFileSizeHigh) shl Int64(32) + Int64(sr.FindData.nFileSizeLow) else result := -1; FindClose(sr); end; but when I stopped calling it the problem at first seemed to go away. Then then came up again. The only place the file is opened is when its created, I made some changes but essentially its the same logic: function ExportSliceToBinFile(MyFileName: String): Integer; var I: Integer; MyOPLToolPathFile: TFileStream; FullFilePath: String; begin Result := 0; if(Not DirectoryExists('tmp')) then CreateDir('tmp'); if(Not DirectoryExists('tmp\out')) then CreateDir('tmp\out'); FullFilePath := 'tmp\out\'+MyFileName; MyOPLToolPathFile := TFileStream.Create(FullFilePath, fmCreate); try try FOR I := 1 TO GlobalToolpathIndex DO begin MyOPLToolPathFile.WriteBuffer(NewOPLToolPath[I], SizeOf(OPL_COMMAND_RECORD)); end; Result := MyOPLToolPathFile.Size; except Result := -1; end; finally FreeAndNil(MyOPLToolPathFile); end; end; this time I pass the size back as the return.. The method that Posts the binary file I used some of your suggestions like this: function TMain.SaveTheSolvedSlice(FileName: String): Boolean; var MimeType, Response, FullFilePath: String; jsonVal: TJSonValue; Stat, Size: String; SendParams: TIdMultiPartFormDataStream; Retries: Integer; begin Result := False; Retries := 0; FullFilePath := 'tmp\out\'+FileName; if (not FileExists(FullFilePath)) then Exit; Response := 'NULL'; HTTPSendFile.Request.Clear; HTTPSendFile.ReadTimeout := 2000; HTTPSendFile.ConnectTimeout := 9000; SendParams := TIdMultiPartFormDataStream.Create; MimeType := 'application/octet-stream'; try SendParams.AddFile('slice', FullFilePath, MimeType).ContentTransfer:='binary'; ActivityPanel.Text := 'HTTP Post Slice'; StatusUpdate(Console, 'Attempting to post file '+FullFilePath+' (size: '+ IntToStr(SOLVED_FILESIZE) +' bytes)'); repeat try Response := HTTPSendFile.Post(STORAGE_OUT, SendParams); except on E: EFOpenError do // I know, this is not the best option, but it is the only way to detect // a sharing violation error with TFileStream. The alternative is to call // CreateFile/FileOpen() and GetLastError() directly, and then create a // THandleStream from the result... StatusUpdate(Console, E.Message); end; Inc(Retries); Delay(250); until (Response <> '') OR (Retries > 10); if(Response <> 'NULL') AND (Response <> '') then begin //{"Slice":{"Status":"ACCEPTED","FileName":"1_Some_Job_Filename.slc","Size":1812}} jsonVal := TJSonObject.ParseJSONValue(Response); try Stat := jsonVal.GetValue<String>('Slice.Status'); Size := jsonVal.GetValue<String>('Slice.Size'); finally jsonVal.Free; end; StatusUpdate(Console, 'Toasted Slice (size: ' + Size + ') ' + Stat.ToLower); Result := (Stat = 'ACCEPTED'); end; finally SendParams.Free; Response := 'NULL'; HTTPSendFile.Response.Clear; end; end; yest I still get this: System.SysGetMem(Opt.out) System._GetMem(???) System._NewUnicodeString(3) System._UStrAsg('',???) System._CopyArray($12F228,$12F3B0,???,5) System._CopyRecord($12F1B0,$12F338,???) IdGlobal.IndyFormat('%s'#$D#$A'Content-Disposition: form-data; name="%s"',(...)) IdMultipartFormData.TIdFormDataField.FormatHeader IdMultipartFormData.TIdFormDataField.GetFieldSize IdMultipartFormData.TIdMultiPartFormDataStream.CalculateSize IdMultipartFormData.TIdMultiPartFormDataStream.IdSeek(???,???) IdGlobal.TIdBaseStream.Seek(???,???) System.Classes.TStream.GetSize IdHTTP.TIdCustomHTTP.PrepareRequest($81ABC10) How do I use SysInternals Process Explorer for open files? I did download this and I have it running. I come form a Linux/Unix world some so of these tools are much different. In linux I can use lsof . I do use two different IdHTTP instances, one for Get and one for Post, that should not hurt anything right? Edited January 19, 2022 by DavidJr. Share this post Link to post
Remy Lebeau 1392 Posted January 20, 2022 21 hours ago, DavidJr. said: I was using this: Fair enough. 21 hours ago, DavidJr. said: but when I stopped calling it the problem at first seemed to go away. Then then came up again. So, then I suggest Process Explorer to check open handles to the file at the time the problem occurs. Maybe something else is opening the file between the time you create it and the time you use it. Antivirus, perhaps? The alternative is to simply not use a file at all. As I demonstrated earlier, it is possible to give TIdMultipartFormStreamStream a TStream for posting. For instance, you could write your data to a TMemoryStream and then Post() that instead of a disk file. 21 hours ago, DavidJr. said: The method that Posts the binary file I used some of your suggestions like this: There is no point in performing the call to TIdHTTP.Post() in a loop that handles EFOpenError, because that exception will never happen. The file is not opened inside that loop, it is opened by the call to TIdMultipartFormDataStream.AddFile(). So, if the file can't be accessed by TIdMultipartFormDataStream, you are not catching that error. 21 hours ago, DavidJr. said: yest I still get this: WHERE do you get that, exactly? That is a call stack for a memory allocation, so again I ask, are you trying to report a memory leak? That is a completely different issue than a file access error. 21 hours ago, DavidJr. said: How do I use SysInternals Process Explorer for open files? Go up to the "Find" menu, choose "Find Handle or DLL...", and enter the name of your data file. You will get back a list of every open handle to the file, if any, including the name of the process that each handle belongs to. 21 hours ago, DavidJr. said: I do use two different IdHTTP instances, one for Get and one for Post, that should not hurt anything right? Correct. Share this post Link to post
DavidJr. 1 Posted January 20, 2022 @Remy Lebeau thanks again. I am not sure if you are asking if am reporting a "memory leak" in the Indy software or in my code? I am not aware of a memory leak anywhere in the code that posts the file using IdHTTP Post. But I will try looking closer with SysInternals Process Explorer, as a last resort use a TStream directly. Share this post Link to post
Remy Lebeau 1392 Posted January 20, 2022 (edited) 12 minutes ago, DavidJr. said: I am not sure if you are asking if am reporting a "memory leak" in the Indy software or in my code? Neither. You keep saying "I get this" and show a call stack for a memory allocation. Why do you keep mentioning that, unless it is causing a problem for you? WHERE and WHEN are you getting that stack trace shown to you? Quote I am not aware of a memory leak anywhere in the code that posts the file using IdHTTP Post. I don't see any memory leak in this code, either. On the other hand, the stack trace you have shown is for a memory allocation made by the 1st call to IndyFormat() inside of TIdFormDataField.FormatHeader() while the HTTP post data is being prepared by TIdHTTP.Post(). But, FormatHeader() appends additional substrings (6, in your case) to the String returned by that 1st IndyFormat() call, so it doesn't make sense why you should be seeing a stack trace for only the 1st memory allocation, and not for other memory allocations (unless you just didn't show them?) Edited January 20, 2022 by Remy Lebeau Share this post Link to post
DavidJr. 1 Posted January 21, 2022 (edited) 22 hours ago, Remy Lebeau said: Neither. You keep saying "I get this" and show a call stack for a memory allocation. Why do you keep mentioning that, unless it is causing a problem for you? WHERE and WHEN are you getting that stack trace shown to you? I don't see any memory leak in this code, either. On the other hand, the stack trace you have shown is for a memory allocation made by the 1st call to IndyFormat() inside of TIdFormDataField.FormatHeader() while the HTTP post data is being prepared by TIdHTTP.Post(). But, FormatHeader() appends additional substrings (6, in your case) to the String returned by that 1st IndyFormat() call, so it doesn't make sense why you should be seeing a stack trace for only the 1st memory allocation, and not for other memory allocations (unless you just didn't show them?) @Remy Lebeau, This morning I went through the thread that processes the data and exports the data to a binary file. Any object created is Freed after use in every method. Also I Set Array lengths back minimal size. I made the following changes: SendParams: TMultipartFormData; //TIdMultiPartFormDataStream; HTTPPostfile: TNetHTTPClient; //IdHTTP; HTTPResp: IHTTPResponse; and that particular problem went away. However now I have a issue with My Habari RabbitMQ client. Not sure its appropriate to post in this thread since its not Indy. But What I will share here and maybe your wisdom can point me in the right direction. The main thread (TForm VCL), I use a timer with interval of 5 seconds, the Timer handler method sets the Timer.Enabled to False until it completes the work of getting a message from the MQ then using the data to get the file, then calls a method from the second thread to process the data and write to binary file. Once its done, the File is posted back to web api. Now is it problematic to use a timer this way? Also the Habari RabbitMQ library is created in the main thread (VLC) and destroyed there. I followed everything according to documentation. Another note to add the code was inherited from a previous developer... the algorithms just work for data processing, the problem is in this app that interacts with network services, I mean no problems seems to come up on methods with the data processing such as Access Violations... I am also requesting from my boss to enlist outside help with this. Thanks. Edited January 21, 2022 by DavidJr. Share this post Link to post
DavidJr. 1 Posted January 21, 2022 My Timer event handler was causing an issue. That is fixed and my code works now. I processed 100 layers and its all good. Calling a destroyed HTTP object is bad too .. my bad. Share this post Link to post
DavidJr. 1 Posted January 28, 2022 (edited) I went back to the TIdHTTP client and I discovered that I get the error again. I will search tomorrow for what could be causing the issue in memory allocation made by the call to IndyFormat(). It always happens after a few successful posts. But I wanted to point out that I am using Delphi Tokyo (10.2.3) and the version of Indy that came with that package. Should I upgrade to a newer version? Edited January 28, 2022 by DavidJr. Share this post Link to post
DavidJr. 1 Posted January 28, 2022 So I verified that there are memory leaks which are listed in the screenshot attached. There are no others that come up with other than these and consistently reproducible when I use IdHTTP Post with use a TIdMultiPartFormDataStream instance local to the procedure. I tried both (not at the same time): SendParams.AddFile('slice', FullFilePath, MimeType).ContentTransfer:='binary'; SendParams.AddFormField('slice', MimeType, '', SolvedSliceStream, FileName).ContentTransfer := 'binary'; So I am going to use MadExcept with this project, but as I posted before is there an issue with the version (of Indy) that came with Delphi/RADStudio 10.2.3 (Tokyo) Enterprise? Or could the issue be with TStream type objects? Share this post Link to post
Remy Lebeau 1392 Posted January 30, 2022 On 1/27/2022 at 6:52 PM, DavidJr. said: I will search tomorrow for what could be causing the issue in memory allocation made by the call to IndyFormat(). It always happens after a few successful posts. But I wanted to point out that I am using Delphi Tokyo (10.2.3) and the version of Indy that came with that package. In Tokyo, IndyFormat() simply calls SysUtils.Format(), so any memory issue will have to be in the RTL itself. Share this post Link to post
Remy Lebeau 1392 Posted January 30, 2022 On 1/28/2022 at 6:39 AM, DavidJr. said: So I verified that there are memory leaks which are listed in the screenshot attached. None of those leaks are related to IndyFormat(). However, you are clearly leaking a TIdHTTP object, which in turn leaks all of its internal objects, as well as the global GIdStack object in the IdStack.pas unit, because GIdStack is reference-counted and all Indy socket components increment its reference count when they are created and decrement it when they are destroyed. Fix the TIdHTTP leak, and the rest of the leaks in that screenshot will disappear. On 1/28/2022 at 6:39 AM, DavidJr. said: So I am going to use MadExcept with this project, but as I posted before is there an issue with the version (of Indy) that came with Delphi/RADStudio 10.2.3 (Tokyo) Enterprise? There are no known issues with memory leaks, no. Share this post Link to post