8

I wrote a Delphi program that extracts and consolidates data from several different spreadsheets of a single .XLS file, to a text file for later processing. It is a Delphi 7 console program.

An excerpt of the most relevant pieces of code will show you that, apparently, my program is pretty well behaved or at least as much as it needs to be.

uses ...  ActiveX, ComObj ... ;

procedure Fatal(s:string);
  ...
  Halt(1);

var ExcelApp:Variant; (* global var *)

begin (* main program block *)
  coInitialize(nil);
  ExcelApp:=CreateOleObject('Excel.Application');
  try
    ExcelApp.Visible:=False;
    ExcelApp.WorkBooks.Open(ExcelFileName);
  ...
    XLSSheet := ExcelApp.Worksheets[ExcelSheetName];
  ...
    try
      XLSRange := XLSSheet.Range[ExcelRangeName];
    except
      Fatal('Range "'+ExcelRangeName+'" not found');
    end;
    if VarIsNull(XLSRange) then Fatal('Range '+ExcelRangeName+' not found');
    for row:=XLSRange.Row to XLSRange.Rows[XLSRange.Rows.Count].Row do
      for col:=XLSRange.Column to XLSRange.Columns[XLSRange.Columns.Count].Column do
        CellValue:=XLSSheet.Cells[Row,Col].Value;
        ...
        if CellValue<>'' then ...
        ...
    ExcelApp.Workbooks.Close;
    ...
  finally
    ExcelApp.Quit;
    coUninitialize;
  end;   
end.

Sometimes, when the program exits, the XLS remains locked. Looking at the Task Manager, I see that Excel.exe process that was started when the client program ran, is still running, eventhoug the client program has exited and succesfully unloaded.

Do you happen to know what are the usual suspects for this behaviour? have any idea where to look for always unloading excel upon client execution?

PA.
  • 28,486
  • 9
  • 71
  • 95
  • Have you tried adding ExcelApp.DisplayAlerts := False; early on? – David Heffernan Sep 06 '12 at 16:48
  • By the way, you need another try/finally block. The call to CoUninitialize can be missed. I know it's frustrating to have nested try/finally but that's what you need here. – David Heffernan Sep 06 '12 at 16:57
  • 1
    @David: Actually, both the calls (CoInitialize and CoUnitialize) should be removed. ComObj calls them both as it's loaded/unloaded, and since this appears to be running in the context of the main thread, the calls are redundant. – Ken White Sep 06 '12 at 17:45
  • @PA. What is the scope of `ExcelApp`? – David Heffernan Sep 06 '12 at 19:21
  • @David, In the barebones test program all the variables are global. – PA. Sep 07 '12 at 06:16
  • 1
    @Ken, if I remove CoInitialize I get a runtime exception. – PA. Sep 07 '12 at 06:16
  • According to Ken there's a Delphi bug relating to the clean up of global variables. But your real program isn't using globals, right? Which Delphi version do you have? – David Heffernan Sep 07 '12 at 06:18
  • yes, are Global vars. I have Delphi 7. It's compiled as a console program, not gui. But, one thing that amazes me is that it only happens erratically, I have not been able to identify when it does and when it doesn't. – PA. Sep 07 '12 at 06:22
  • Well, I'd want reproducability. And I'd stop using globals and put the code inside a procedure. That will mean that tidy up happens when the now local ExcelApp goes out of scope. At the end of that procedure. – David Heffernan Sep 07 '12 at 06:33
  • If removing CoInitialize from where you're showing it in your code gives you an exception, you need to post a bunch more code (and more details you didn't provide in the first place, like the fact it's a console app). Is it also a multi-threaded app? If so, where does the code you show excecute (main or secondary thread)? What does `Fatal` do exactly? – Ken White Sep 07 '12 at 11:02
  • I think you need to call CoInitialize. I don't see that using ComObj results in CoInitialize being called. I set a breakpoint on ComObj.InitComObj. It doesn't fire. I set a breakpoint in the ComObj finalization. It fires, but NeedToUninitialize is false. – David Heffernan Sep 07 '12 at 11:31
  • 1
    @David - It's called from 'TApplication.Initialize' (comboj.pas sets the InitProc). Won't be called for a console app. – Sertac Akyuz Sep 07 '12 at 12:20
  • @SertacAkyuz Thank you. That clears it up! That's why PA. needs to call it, but Ken doesn't. – David Heffernan Sep 07 '12 at 12:27
  • Edited my question with some important information. It's a Console app. ExcelApp is a Global Var. The code is the main block. Fatal is a procedure that ends the program with a call to Halt. I believe that those are the pieces that make the program exhibit such behavior. – PA. Sep 07 '12 at 13:17

3 Answers3

6

You need to release the ExcelApp variant. It still holds a reference count of 1, and therefore Excel isn't completely closed.

Add this to your code (the marked line):

finally
  ExcelApp.Quit;
  ExcelApp := Unassigned;        // Add this line
  coUninitialize;
end;  

Here is some simple code to reproduce the problem, and test the solution:

// Add two buttons to a form, and declare a private form field. 
// Add OnClick handlers to the two buttons, and use the code provided. 
// Run the app, and click Button1. Wait until Excel is shown, and then click
// Button2 to close it. See the comments in the Button2Click event handler.
type
  TForm1=class(TForm)
    Button1: TButton;
    Button2: TButton;
    procedure Button1Click(Sender: TObject);
    procedure Button2Click(Sender: TObject);
  private
    ExcelApp: Variant;
  end;

implementation

uses
  ComObj;

procedure TForm1.Button1Click(Sender: TObject);
begin
  ExcelApp := CreateOleObject('Excel.Application');
  ExcelApp.Visible := True;
end;

procedure TForm1.Button2Click(Sender: TObject);
begin
  ExcelApp.Visible := False;
  ExcelApp.Quit;

  // Leave the next line commented, run the app, and click the button.
  // After exiting your app NORMALLY, check Task Manager processes, and you'll
  // see an instance of Excel.exe still running, even though it's not
  // in the Applications tab. 
  //
  // Do an "end process" in Task Manager to remove the orphaned instance 
  // of Excel.exe left from the above. Uncomment the next line of code
  // and repeat the process, again closing your app normally after clicking
  // Button2. You'll note that Excel.exe is no longer in
  // Task Manager Processes after closing your app.

  // ExcelApp := Unassigned;
end;

end.
Ken White
  • 123,280
  • 14
  • 225
  • 444
  • @David: It will. I've run into this precise problem in the past; it's not just a guess. You could always try it, though; the poster provided enough code to both replicate the problem and try my solution. :-) – Ken White Sep 06 '12 at 16:54
  • I always thought that the Excel process died at the call to `Quit`. Is that not the case? – David Heffernan Sep 06 '12 at 16:58
  • No. :-) It's a particularly nasty problem if your app crashes for some reason; when you try and run it again, you get errors that seem to be out of place, until you check Task Manager and find Excel.exe in the 'processes' (not Applications!) tab and do an end process. The errors then stop. :-) As I said, I wasn't just guessing. – Ken White Sep 06 '12 at 17:05
  • 1
    Well, if you are correct, then the issue must be whether or not the reference is released before or after the call to `CoUninitialize`. Because the reference will be released. You are just controlling when. – David Heffernan Sep 06 '12 at 17:10
  • @David, possibly. Except when your app has already crashed, closed totally, and Excel remains orphaned in the TM Processes tab, it apparently doesn't get released (otherwise, how would it still be there?). – Ken White Sep 06 '12 at 17:47
  • `ExcelApp := UnAssigned;` will release the reference or when ExcelApp goes out of scope. We cannot see the scope of ExcelApp here, so I guess setting it to unassigned will do the job. – LU RD Sep 06 '12 at 17:47
  • 1
    @LURD: You're right, of course. `nil` worked in earlier versions of Delphi, before the variants stuff was moved out into it's own unit (D6?). `Unassigned` is the proper way. Corrected, and thanks. :-) – Ken White Sep 06 '12 at 17:51
  • My experiments back you up to a degree. `ExcelApp := Unassigned` doesn't kill the Excel process. However, exiting the procedure that declared the `ExcelApp` variable does the trick. But to make that work you need the `CoInitialize`/`CoUninitialize` moved outside the procedure that declares `ExcelApp` variable. – David Heffernan Sep 06 '12 at 18:00
  • @David: Posted complete test code so you can both produce the problem and test the solution provided. Also, as I said in my comment to the question itself, CoInitialize/CoUninitialize should be removed entirely, as they're called automatically in `ComObj`. – Ken White Sep 06 '12 at 18:01
  • Well, your program doesn't behave the way the comment says it does when I run it. I also can't see where in ComObj the code calls `CoInitialize`. Never mind. – David Heffernan Sep 06 '12 at 18:13
  • @David: `ComObj` has an `initialization` section that assigns `@InitComObj` to `InitProc`, which is called by the RTL at initialization of the unit. `InitComObj` (just above the initialization section) calls the prior `InitProc` and then either `CoInitializeEx` or `CoInitialize` and sets a boolean `NeedsToUnitialize` flag. The `finalization` section calls `CoInitialization` if the `NeedToUnitialize` flag was set. (Based on the D2007 version of ComObj - XE2 works the same, although there's code added between `InitComObj` definition and the `initialization` keyword.) – Ken White Sep 06 '12 at 18:41
  • Yes I see. Strangely on my XE2, I can't get the debugger to stop on InitComObj. – David Heffernan Sep 06 '12 at 18:47
  • 1
    I understand what's going on now. Your example is bogus. You are killing the process before it gets a chance to clean up `ExcelApp`. Of course that leaves Excel running. But there's not a lot that can be done about that. In the code in the question, let's assume that code is contained in a procedure. When that procedure ends, then `ExcelApp` is cleaned up and then Excel goes away. Adding the assignment to `Unassigned` does nothing because the compiler introduces a hidden implicit managed variable that refers to the COM object. – David Heffernan Sep 06 '12 at 19:19
  • What I mean by that last statement is what is discussed here: http://stackoverflow.com/questions/7759081/is-the-compiler-treatment-of-implicit-interface-variables-documented – David Heffernan Sep 06 '12 at 19:20
  • 1
    @David: "Bogus?" No, it's not, and your phrasing is a little rude. I'm closing the application normally, after clicking a button. In what way is that "killing the process"? It goes through the normal application messaging that clicking the 'x' in the top right corner goes through. Adding the assignment to `Unassigned` makes the issue go away. I never said I was reproducing the **exact** code used in the original question; I produced an example of the same **behavior**. – Ken White Sep 06 '12 at 19:46
  • @David: Ah, I see. You didn't read the comments in `Button2Click` correctly, and misinterpreted what I was saying in the instructions. Added some more details to improve comprehension (hopefully). – Ken White Sep 06 '12 at 19:49
  • Yeah, "bogus" is inflammatory. Sorry. But your answer is still not helpful or accurate. I'm compiling in XE2. With that line commented out, click Button1, Excel process starts, click Button2, Close app, Excel process stops. With the line commented back in, click Button1, Excel process starts, click Button2, Excel process stops. And that's exactly what you would expect. You see, with the line commented out, ExcelApp goes out of scope as the program closes. And then Excel closes. That's how it should work. If you see something different, somethings broken on your system. – David Heffernan Sep 06 '12 at 19:58
  • The bottom line is that when the last reference to the COM object goes out of scope, the Excel process terminates. Now, you were bang on about that bit. And I was talking piffle when I thought `Quit` would terminate process. But there's nothing in the question that would lead one to think that the reference counting on the COM object is mis-managed. At some point the `ExcelApp` in the Q will get cleaned up, and Excel will be terminated. Do you see where I'm coming from? Sorry for talking such a long time to express myself. – David Heffernan Sep 06 '12 at 20:00
  • @David: It doesn't work that way in Delphi 2007, and I tested the code before posting it, and it's a relatively fresh install of both Win7 and D2007. I can reproduce it repeatedly, and just copied the executable to a machine that doesn't have Delphi installed but has Office 2007 (like mine) and can repeat it there. Maybe a change in XE2? As far as my answer not being helpful, I guess we'll have to wait for the poster to determine that; there's no indication of which version of Delphi or Excel is being used, so maybe the issue I have is present there as well. – Ken White Sep 06 '12 at 20:02
  • Yeah, I'm sure you can reproduce it. But do you agree with my analysis? Namely that you don't have to explicitly set managed types to `nil` or `Unassigned`, and you can just wait until the last reference goes out of scope. In which case isn't D2007 suffering from a bug? Of course, if OP is using the bugged version, then your answer would be the solution! – David Heffernan Sep 06 '12 at 20:05
  • Yes, that's usually the case. That's why I was surprised when the issue with Excel first arose (my first comment to you in this answer). The fact I can repeatedly reproduce it means that there's either something wrong in Excel or in Delphi (may be an Excel issue - see deleted answer from Lloyd) that's screwing up the reference counting and leaving orphaned instances running. (I can create multiple orphaned instances by running the app multiple times with the `Unassigned` line commented out and exiting each run normally. Each one leaves a new instance of Excel behind.) – Ken White Sep 06 '12 at 20:08
  • 1
    @Ken, I just had a look in one of my D2007 applications (running 24h/day generating excel reports 100% uptime). All ExcelApps are locally declared and all ends with quit and unassigned. If my memory is correct, this was the only way to close excel properly. It might have hanged since, but your answer stands. – LU RD Sep 06 '12 at 20:19
  • @LURD: Thanks. I don't remember what version of Delphi I first ran into this with; I don't do much Excel automation any more, because we use XLSReadWrite and just read or write directly to the file without Excel being involved. :-) – Ken White Sep 06 '12 at 20:24
  • We do use XLSReadWrite as well, but for printing sheets we had to go for automation (at least this was the case when software was written). – LU RD Sep 06 '12 at 20:31
  • It sure would be nice to understand this. Because if this is what you need to do, something is very badly broken. – David Heffernan Sep 06 '12 at 20:32
  • @David: It's a known problem. A member of Team B, Deborah Pate, who was one of the early "expert* references for Office automation, even mentions it on [her web pages](http://www.djpate.freeserve.co.uk/AutoExcl.htm), which were last updated around Delphi 5. (See the note at the end of the section on "Starting Excel (late binding)" about a quarter of the way down the page, a couple of times in the "Closing Excel" topic just below that, etc.) It's a long-existing problem. (I think Deborah's pages provided the solution when I first ran into it.) – Ken White Sep 06 '12 at 23:17
  • That fails to explain why program termination won't take the vars out of scope. I still have a strong dislike for your answer. Nowhere do you give any justification for what you claim. And nowhere do you state that there is a Delphi bug relating to reference clean up at program termination time. – David Heffernan Sep 07 '12 at 05:46
  • @David, did you even bother to look at Deborah's site I linked to? She mentioned the issue in **Delphi 5**, and mentions it on **both** the Excel and Outlook pages (but not Word, for some reason) when discussing shutting them down when using late-binding. You're asking me to explain the internals of two MS Office apps and their COM implementations, and I'm afraid i can't do that; I don't work on those internals. I've told you what I know: It's a long documented issue, I've run into it, I reproduced one version of it easily above, and both LU RD and a pretty-well known TeamB member did too. – Ken White Sep 07 '12 at 10:57
  • @David, and the three of us all found that setting the variant references to Unassigned before exiting solved the problem. I'm sorry you don't like that (sorry - *strongly dislike*), but it's not as if I'm making it up. It's a **documented** issue; if you check, you can probably find it in QC. (And I'm not checking, because as far as I'm concerned it's been confirmed; as I said, I've personally experienced it, and two others have said they have as well.) – Ken White Sep 07 '12 at 11:00
  • 1
    Yes I read that page. It says that you need to clear all references to the COM object. But you don't need explicitly to clear managed types to do that. That happens automatically when a variable goes out of scope. Let's go right back to basics. Do you agree with the statement that managed types are cleared automatically when they go out of scope? – David Heffernan Sep 07 '12 at 11:09
  • @David, no. The article explicitly says you need to set the COM references to **Unassigned**, and the posted code in the article clearly shows that as well. I agree that managed types **should** be automatically cleared; I did that 10/12 comments ago. But in this case, **they are not** being cleared. It's documented that they don't, and that extra efforts need to be made in order to do so. Since it appears only to affect Excel and Outlook (but no reference is made on Pate's site to Word), it appears to be an issue with them specifically. I can't explain the bug; I can provide the workaround. – Ken White Sep 07 '12 at 11:13
  • Well, I don't agree with what you are saying. I don't agree with your reading of Deborah's site. I don't agree that if 3 people believe one thing, and only one person believes something else, that the 3 people must be right. I can't get my Delphi 6 to behave the way you describe. In my D6 your code in this answer doesn't behave the way you describe. I think we are done. You believe one thing. I am not convinced. – David Heffernan Sep 07 '12 at 11:24
  • @David: How can you not agree with "Excel may be running invisibly, even after you think you've freed it, if any of your Workbook or Worksheet variables are still 'live'." and "Note, however, that Excel will hang around in memory, running invisibly, unless you've released all your workbook and worksheet variables. Disconnect any components, **set any interface variables to nil**, and **set any variants to Unassigned** to prevent this.". (Taken from the Excel pages at Deborah's site verbatim.) But OK, I give up. You don't agree with what I wrote, despite my providing evidence and code. – Ken White Sep 07 '12 at 11:40
  • I don't agree with your interpretation of that. You don't need to explicitly clear ExcelApp. The compiler does it for you. – David Heffernan Sep 07 '12 at 11:42
  • @David: **and set any Variants to Unassigned** seems pretty hard to misinterpret, and I'll post a screen cap when I get back home tonight as an edit to my post of a few orphaned Excel instances that proves that the compiler doesn't (or that Excel doesn't properly behave). But you obviously stopped listening, so I think I'm done here. If my answer is wrong, downvote it. If you just disagree, you've said so. Either way, let's just agree to disagree and move along. I'll stay with the working solution when needed in my code. :-) – Ken White Sep 07 '12 at 12:38
  • Why does the answer not behave the way you say it does when I run it? You can see why I am sceptical. – David Heffernan Sep 07 '12 at 12:44
  • After some testing I have accepted Ken's answer. It does fix my problem. At least for the combination of compiler version, type of application, and code. Also, the answer explains why the code was not freeing Excel, and helped me to deduct the reason why it happened some times and not some others (thru the calling of Fatal). – PA. Sep 07 '12 at 13:27
  • @David, as I said, I can post a screen capture later (Can't do it from here; the proxy won't let me.) What versions of Delphi and Excel are you using? – Ken White Sep 07 '12 at 13:39
  • @PA. Perhaps you should stop using Halt! – David Heffernan Sep 07 '12 at 13:43
  • 1
    Yes, using Halt is the problem here. Stop doing that. – David Heffernan Sep 07 '12 at 13:50
  • All versions of Delphi, Excel 2007 and Excel 2010 – David Heffernan Sep 07 '12 at 14:33
4

I have encountered much the same problem in XE2 and my solution was to replace such code samples:

fExcel.ActiveWorkBook.ActiveSheet.Range[
    fExcel.ActiveWorkBook.ActiveSheet.Cells[3, 2],
    fExcel.ActiveWorkBook.ActiveSheet.Cells[3+i,1+XL_PT_Tip_FieldCount]
].Formula := VarArr;

with:

cl := fExcel.ActiveWorkBook.ActiveSheet.Cells[3, 2];
ch := fExcel.ActiveWorkBook.ActiveSheet.Cells[3+i,1+XL_PT_Tip_FieldCount];
fExcel.ActiveWorkBook.ActiveSheet.Range[cl, ch].Formula := VarArr;

Same happens in this case, where sheet variable is used:

sheetDynamicHb := fExcel.ActiveWorkBook.Sheets['Dynamics Hb'];
cl := sheetDynamicHb.Cells[52, 2];
ch := sheetDynamicHb.Cells[52+i, 2+3];
sheetDynamicHb.Range[cl, ch].Formula := VarArr;

Somehow introducing temp variables (cl,ch: Variant) does the trick. It seems like the nested Excel variable access does something odd. I can not explain why this works like that, but it does work..

Kromster
  • 7,181
  • 7
  • 63
  • 111
4

I faced the same issue trying to close "zombie" Excel processes (the ones that stay running if I launch them from my app and then forced terminate the app). I tried all suggested actions with no luck. Finally I created a combined killer procedure that robustly does the trick using WinApi if usual COM methods do not help.

procedure KillExcel(var App: Variant);
var
  ProcID: DWORD;
  hProc: THandle;
  hW: HWND;
begin
  hW := App.Application.Hwnd;
  // close with usual methods
  App.DisplayAlerts := False;
  App.Workbooks.Close;
  App.Quit;
  App := Unassigned;
  // close with WinApi
  if not IsWindow(hW) then Exit; // already closed?
  GetWindowThreadProcessId(hW, ProcID);
  hProc := OpenProcess(PROCESS_TERMINATE, False, ProcID);
  TerminateProcess(hProc, 0);
end;
Fr0sT
  • 2,959
  • 2
  • 25
  • 18