Internal AV , fix?

h.hasenack · 2012-10-10 09:15:30

in function TServiceFactoryClient.Invoke and AV is raised when my Interface for a dynamic rest server is destroyed (CLient side). This AV is catched by

I have added this check to get around the internal AV.

  // compute URI according to current routing scheme
  if Assigned(FClient) and Assigned(fClient.Model) then
    uri := fClient.Model.Root+'/'
  else Exit;

Anyway, I feel any try .. except block should at least re-raise low level system errors like AV's and alike, because if you don't these can cause very bad unexpected effects on other parts of the app.

destructor TInterfacedObjectFake.Destroy;
begin
  if (fFactory<>nil) and (fFactory.InstanceCreation=sicClientDriven) and
     (fClientDrivenID<>0) then // fClientDrivenID=0 if no instance used
  try // release server instance
    fFactory.NotifyInstanceDestroyed(fClientDrivenID);
  except
    ; // ignore any exception here
  end;
  inherited;
end;

Usually these kind of "catch all" except blocks look like this in my app:

  try 
    dosomethingdangerous;
  except
    if IsFatalException(ExceptObject) then
      raise;
  end;

The IsFatalException routine looks something like this:

const
//List of standard Delphi exceptions that will raise a FatalChainedException
cDefaultFatalExceptions : Array [0..16] of TClass =
      (EOutOfMemory, ERangeError, EIntOverflow, EInvalidOp,
        EOverflow, EUnderflow, EInvalidPointer, EInvalidCast,
        EConvertError, EAccessViolation, EPrivilege,
        {$WARN SYMBOL_PLATFORM OFF}
        EAbstractError,
        {$WARN SYMBOL_PLATFORM ON}
        {$WARN SYMBOL_DEPRECATED OFF}
        EStackOverflow, EWin32Error,
        {$WARN SYMBOL_DEPRECATED ON}
        EIntfCastError, ESafecallException, EInvalidArgument
      );

{ Determine if an Object is a Fatal exception
   An object can be qualified as fatal in several different ways:
   - a ChainedException or one of its InnerExceptions is a FatalException
   - The object-class is listed in vFatalExceptions
   - one of the methods in vFatalExceptionDetectors qualifies the object as fatal
}
function IsFatalException(E: TObject): Boolean;
var
  i: Integer;
  Addr:pointer;
begin
  result := false;
  if Assigned(E) then
  begin
    if not (E is TObject) then
      Result:=True // if the exception is not derived from TObject, it's a fatal exception
    else if not (E is Exception) then
      Result:=True // if the exception is not derived from exeption, it's a fatal exception
    else if (E.ClassType=Exception) then
      Result:=True // if the exception object is directly Exception class without being derived from Exception, it's a fatal exception
    else if E is EChainedException then
      Result:=EChainedException(E).Fatal  // this will actually recurse
    else
    begin // an exception derived from exception
      while not Result and Assigned(E) do
      begin
        // pass by list of fatal exception classes
        for i := Low(vFatalExceptions) to High(vFatalExceptions) do
        begin
          if (E is vFatalExceptions[i]) then
          begin
            Result := true;
            break;
          end;
        end;
        // pass by list of fatal exception detection handlers
        for i:=low(vFatalExceptionDetectors) to high(vFatalExceptionDetectors) do
        begin
          vFatalExceptionDetectors[i](E,Result);
          if Result then
            Break;
        end;
        if not Result then
          GetInnerExceptionInfo(E,E,Addr);
      end;
    end;
  end;
end;

initialization

  RegisterFatalExceptions(cDefaultFatalExceptions);

ab · 2012-10-10 09:54:40

IMHO releasing client-side service instances after the main client is destroyed is... a bit strange...

You are right, the "except ; end" is a bit rough.
Your IsFatalException() is a nice trick.
But in fact, notifying the server is optional, since there is a timeout auto-release of client sessions on the server side.
You A/V is raised because your main client is already destroyed, which should not be the case, AFAIK.
In the meanwhile, manual proper check of Client is to be added in your code: use the client-side service interface instances only if the client is still there.
Should not be the more appropriate implementation be a "Zeroing" feature, which would set all interface variables to nil when the client is removed?
It could be possible, but I suspect

h.hasenack · 2012-10-10 10:21:11

Ah, I found the culprit. Indeed I forgot to 'kill' one interface in my UnitTest Teardown routine.
In my Unit tests I insist on actively terminating client sessions, just to make sure.

The nice thing of the IsFatalExceptionb is that it also checks the sub-exception (if any) so a AV catched and next raised with RaiseOuterException with even EABort would still be a fatal exception.

  try
    try
      ProduceAccessViolation
    except
     on E:Exception do
       E.RaiseOuterException(EAbort.Create('Aborted'));
    end
  except
    if IsFatalException(ExceptObject) then // Will be true because EAbort is raised because of an aV
      .... 
  end;

ab · 2012-10-10 12:02:44

IMHO EConvertError or ERangeError is not a "fatal" exception, in some cases.
And what is EChainedException? This is not a standard type, as far as I found out.

I would rather check for "safe" exceptions and ignore them.

h.hasenack · 2012-10-10 12:55:31

Yes, some of them are quite subjective
Inmy world ERangeError is only raised by compiler range checking. SO I decided it to be "Fatal"
The EConvertError is indeed very open for discussion .

EChainedException is a special exception I have created myself to allow "automatic" raiseouterexecption behaviour. Thus, when a EChainedexception is Created/raised, and another exception is already 'active' this one gets to be "owned" by the Echainedexception so I can put "high level" error messages without losing the low level exception information. The subexception object is destroyed togehter with the chainedexception object. (I had this already in D6, way before Emb. decided to implement RaiseOuterException)

EChainedException itself should NOT be raised, but always as a derived one. Therefore the (underived) EChainedException itself is a fatal.

Last edited by h.hasenack (2012-10-10 12:56:44)

ab · 2012-10-15 06:36:21

I've added re-raise of minimal number of exceptions in the method.
See http://synopse.info/fossil/info/1f7ac20b82

mORMot Open Source

#1 2012-10-10 09:15:30

Internal AV , fix?

#2 2012-10-10 09:54:40

Re: Internal AV , fix?

#3 2012-10-10 10:21:11

Re: Internal AV , fix?

#4 2012-10-10 12:02:44

Re: Internal AV , fix?

#5 2012-10-10 12:55:31

Re: Internal AV , fix?

#6 2012-10-15 06:36:21

Re: Internal AV , fix?

Board footer