SSH/SFTP Server Issue with createevent (fails with error 161)

[tcaduto12068]

Hi,
I use the SSH/SFTP server components for a small managed SFTP server and it has been working flawlessly for over a month and half. I run it as a win32 service on windows server 2012 and the memory usages is stable, it hovers around 5mb according to the resource monitor, it does go up when really busy to 7 or 8 mb, but then goes back down to 5.

So last night around 8:29pm one of the biggest users could not upload files, they could logon and disconnect but when they tried to upload the log recorded this:

Code: Select all

Cannot create event: Error Code 161, Error MSG:The specified path is invalid.

This was raised in the "SSHServerClientError" event of the SSH server.

Restarting the service cleared the issue, so I am thinking the handle from the createevent is maybe not being closed and the process is running out of handles after a long time?

About a month and a half ago I got a similar error when testing on my laptop (which has way less resources than the server) and I was able to get a create event error using filezilla with a bunch of concurrent uploads.

I believe I modified the createevent code in ssh server a bit so it would show the error message as well as the last error code.

Any ideas on how to track this issue down?

[tcaduto12068]

I tracked this down in the SBridge source and it's in ScVio in the constructor TAsyncThread.Create on or around line 356.

UPDATE: THIS does not work, don't change anything
I think you guys should maybe look at changing it to this:

Code: Select all

  lasterrorcode:=0;
  FDataAvailable := TEvent.Create(nil, True, False, '');
{$IFDEF MSWINDOWS}
  if not Assigned(FDataAvailable.Handle) then
    begin
         lasterrorcode:=GetLastError;
         if lasterrorcode <> 0 then
            raise Exception.Create(format('%s: Error Code %d, Error MSG:%s',[SCannotCreateEvent,lasterrorcode,SysErrorMessage(lasterrorcode)]));
    end;
{$ENDIF}  

I am guessing that evaluating GetLastError without checking if a handle was actually assigned is the culprit as it was picking up some other error the thread generated i.e. "The specified path is invalid." which seems to have nothing to do with creating handles.

[ViktorV]

The similar question has already been discussed on our forum. Follow the link viewtopic.php?t=34142

[tcaduto12068]

Hi Victor,
That other thread was more about the backlog being set too small.

This issue is with the fact that GetLastError is being called without checking the status of the object created by tevent, the way it was coded it could have been picking up some other error not related to the event creation. I know the windows API docs say GetLastError is for the calling thread but it's still possible that it could pick up a unrelated error.

[ViktorV]

Thank you for being interested in our products.
We will consider the possibility to change this behavior of SecureBridge in one of the next releases.

[tcaduto12068]

Just a FYI,
I reported the event.create issue on the FPC mailing list and it appears to be caused by a serious bug in the FPC RTL.

See the report here:
http://bugs.freepascal.org/view.php?id=30747

The work around with the GUID as the name param for Tevent does work without having to recompile the RTL.

Problem is in:
intBasicEventCreate in $fpcdir/rtl/win/systhrd.inc

The call to createevent in intBasicEventCreate had this: pchar(name).
So if name was set in tevent to '' this would cause a null char #0 to be sent instead of NIL.
Which caused the error 161 with a lot of tevent create and destroy.

I recompiled the RTL and used the fix in the bug report as provided by one of the FPC devs from the mailing list and the error did go away.

This is a pretty serious bug in a busy system and would cause AVs all over the place.

I guess you guys could just leave your code the way it is or maybe create your own implementation of Tevent that does not use the FPC RTL Or Delphi RTL and instead calls createevent directly.

[ViktorV]

Thank you for being interested in our products and your help on issue investigation.
We will investigate the behavior of SecureBridge according to your description and inform you about the results.

[tcaduto12068]

Just a FYI, Almost all versions of Delphi (except the most recent versions) have the same issue, please see this link: http://qc.embarcadero.com/wc/qcmain.aspx?d=100175

and this one:

http://qc.embarcadero.com/wc/qcmain.aspx?d=126105

[ViktorV]

Thank you for your advice.

[ViktorV]

We changed the error message "Cannot create event" format. The new build of SecureBridge 7.2.3 including this change is already available for download.

[tcaduto12068]

Hi Victor,
Did you find a way to get around the createevent bug in the older Delphi and FPC runtime libs?
Or did you just change the error message?

[ViktorV]

Changes in SecureBridge 7.2.3 involved the code change:

Code: Select all

  if GetLastError <> 0 then
    raise Exception.CreateFmt(SErrEventCreate, ['']);

на

Code: Select all

  if FevReady.Handle = 0 then begin
    LastErrorCode := GetLastError;
    if LastErrorCode <> 0 then
      raise Exception.CreateFmt(SCannotCreateEvent, [SysErrorMessage(LastErrorCode), LastErrorCode]);
  end;

[tcaduto12068]

Hi Victor,

That will not prevent the error on versions of Delphi and Free Pascal that have the create event bug.

for example in TSshChannel.Create(and any other proc/func that uses tevent.create) the problem is this:

FevReady := TEvent.Create(nil, True, False,'');

The problem is the underlying compilers use of the API createevent and the name if blank must be passed as a null value i.e. nil.

It's only been fixed at the compiler runtime level in the very newest versions of Delphi and the next release of FPC.

The only way to fix it for older compilers is to pass a unique name for the tevent.create and the only thing that I found to work is a GUID:

What you could do is if tevent.create fails with a name of '' then call it again with a GUID for the tevent.create name parm.

What you could do is something like this maybe?

{$IFDEF MSWINDOWS}
if GetLastError <> 0 then
begin
if CreateGUID(guid) = 0 then
name:=GUIDToString(guid) else name:='';
FevReady := TEvent.Create(nil, True, False,name);
if GetLastError <> 0 then
raise Exception.CreateFmt(SErrEventCreate, ['']);
end;
raise Exception.CreateFmt(SErrEventCreate, ['']);
{$ENDIF}

[ViktorV]

We will explore the possibility of changing this behavior of SecureBridge for older versions of the IDE. But so far, we haven't planned this implementation, since everything works fine in the new versions of the IDE.

[tcaduto12068]

ok, only reason I think it's important is not everyone uses the very latest Delphi IDE or FPC compiler version, and this createevent bug is pretty serious anytime your using it with threads or using it a lot.
With the bug the sftp server becomes unstable really fast.