David Heffernan
Posts: 9
Joined: 2013-01-03
|
There are many instances in the Add-In Express code base which re-raise exceptions in an erroneous manner. For instance, in adxAddIn consider this method:
constructor TadxEventSink.Create(AddInModule: TadxCOMAddInModule; AControl: TadxCommandBarControl);
begin
inherited Create;
FAddInModule := AddInModule;
FIControl := AControl;
FIBuiltInControl := nil;
try
if AControl.DefaultInterface.Type_ = msoControlButton then
InterfaceConnect(AControl.DefaultInterface, DIID__CommandBarButtonEvents, Self, FCookie)
else
InterfaceConnect(AControl.DefaultInterface, DIID__CommandBarComboBoxEvents, Self, FCookie);
except
on E:SysUtils.Exception do begin
{$IFDEF UNICODE}
OutputDebugString(PWideChar('TadxEventSink.Create1 error: ' + E.Message));
{$ELSE}
OutputDebugString(PAnsiChar('TadxEventSink.Create1 error: ' + E.Message));
{$ENDIF}
raise E;
end;
end;
end;
In this code, the exception is deemed to be handled by the except block, and when execution leaves the except block, the exception is destroyed. However, the use of raise E will also raise that same object. Which means that when it is caught higher up, it will be destroyed again. The classic double free.
Marc Durdin discusses this issue in more detail here: https://marc.durdin.net/2012/10/how-not-to-re-raise-an-exception-in-delphi/
You need to use the special syntax of raise that is provided for re-raising exceptions: http://docwiki.embarcadero.com/RADStudio/en/Exceptions#Re-raising_Exceptions
So, change
raise E;
to
raise;
As I said, this mistake is made many times in the source code. |
|
Andrei Smolin
Add-in Express team
Posts: 18819
Joined: 2006-05-11
|
Huge thanks, David!
We will change the code.
Andrei Smolin
Add-in Express Team Leader |
|
David Heffernan
Posts: 9
Joined: 2013-01-03
|
|
Andrei Smolin
Add-in Express team
Posts: 18819
Joined: 2006-05-11
|
You are welcome!
Andrei Smolin
Add-in Express Team Leader |
|