From b64a774b2fbfab034c0b7fff1641a46d8123d19f Mon Sep 17 00:00:00 2001 From: Jens Geyer Date: Wed, 23 Jan 2013 20:58:47 +0100 Subject: [PATCH] THRIFT-1842 Memory leak with Pipes Patch: Jens Geyer --- lib/delphi/src/Thrift.Serializer.pas | 1 - lib/delphi/src/Thrift.Transport.Pipes.pas | 140 +++++++++++----------- lib/delphi/test/TestClient.pas | 4 + 3 files changed, 77 insertions(+), 68 deletions(-) diff --git a/lib/delphi/src/Thrift.Serializer.pas b/lib/delphi/src/Thrift.Serializer.pas index a81cef08..dce68639 100644 --- a/lib/delphi/src/Thrift.Serializer.pas +++ b/lib/delphi/src/Thrift.Serializer.pas @@ -138,7 +138,6 @@ procedure TSerializer.Serialize( const input : IBase; const aStm : TStream); // Serialize the Thrift object into a byte array. The process is simple, // just clear the byte array output, write the object into it, and grab the // raw bytes. -var iBytes : Int64; const COPY_ENTIRE_STREAM = 0; begin try diff --git a/lib/delphi/src/Thrift.Transport.Pipes.pas b/lib/delphi/src/Thrift.Transport.Pipes.pas index 76ed93bc..66db240a 100644 --- a/lib/delphi/src/Thrift.Transport.Pipes.pas +++ b/lib/delphi/src/Thrift.Transport.Pipes.pas @@ -196,8 +196,7 @@ type protected function AcceptImpl: ITransport; override; - - function CreateNamedPipe : Boolean; + procedure CreateNamedPipe; // INamedServerPipe function Handle : THandle; @@ -599,31 +598,36 @@ begin result := FALSE; sd := PSECURITY_DESCRIPTOR( LocalAlloc( LPTR,SECURITY_DESCRIPTOR_MIN_LENGTH)); - Win32Check( InitializeSecurityDescriptor( sd, SECURITY_DESCRIPTOR_REVISION)); - Win32Check( SetSecurityDescriptorDacl( sd, TRUE, nil, FALSE)); + try + Win32Check( InitializeSecurityDescriptor( sd, SECURITY_DESCRIPTOR_REVISION)); + Win32Check( SetSecurityDescriptorDacl( sd, TRUE, nil, FALSE)); - sa.nLength := sizeof( sa); - sa.lpSecurityDescriptor := sd; - sa.bInheritHandle := TRUE; //allow passing handle to child + sa.nLength := sizeof( sa); + sa.lpSecurityDescriptor := sd; + sa.bInheritHandle := TRUE; //allow passing handle to child - if not CreatePipe( hCAR, hPipeW, @sa, FBufSize) then begin //create stdin pipe - Console.WriteLine( 'TServerPipe CreatePipe (anon) failed, '+SysErrorMessage(GetLastError)); - Exit; - end; + if not CreatePipe( hCAR, hPipeW, @sa, FBufSize) then begin //create stdin pipe + Console.WriteLine( 'TServerPipe CreatePipe (anon) failed, '+SysErrorMessage(GetLastError)); + Exit; + end; - if not CreatePipe( hPipe, hCAW, @sa, FBufSize) then begin //create stdout pipe - Console.WriteLine( 'TServerPipe CreatePipe (anon) failed, '+SysErrorMessage(GetLastError)); - CloseHandle( hCAR); - CloseHandle( hPipeW); - Exit; - end; + if not CreatePipe( hPipe, hCAW, @sa, FBufSize) then begin //create stdout pipe + Console.WriteLine( 'TServerPipe CreatePipe (anon) failed, '+SysErrorMessage(GetLastError)); + CloseHandle( hCAR); + CloseHandle( hPipeW); + Exit; + end; - FClientAnonRead := hCAR; - FClientAnonWrite := hCAW; - FReadHandle := hPipe; - FWriteHandle := hPipeW; + FClientAnonRead := hCAR; + FClientAnonWrite := hCAW; + FReadHandle := hPipe; + FWriteHandle := hPipeW; - result := TRUE; + result := TRUE; + + finally + if sd <> nil then LocalFree( Cardinal(sd)); + end; end; @@ -647,9 +651,7 @@ end; function TNamedServerPipeImpl.AcceptImpl: ITransport; var connectRet : Boolean; begin - if not CreateNamedPipe() - then raise TTransportException.Create( TTransportException.TExceptionType.NotOpen, - 'TServerPipe CreateNamedPipe failed'); + CreateNamedPipe; // Wait for the client to connect; if it succeeds, the // function returns a nonzero value. If the function returns @@ -686,7 +688,7 @@ begin end; -function TNamedServerPipeImpl.CreateNamedPipe : Boolean; +procedure TNamedServerPipeImpl.CreateNamedPipe; var SIDAuthWorld : SID_IDENTIFIER_AUTHORITY ; everyone_sid : PSID; ea : EXPLICIT_ACCESS; @@ -698,50 +700,54 @@ const SECURITY_WORLD_SID_AUTHORITY : TSIDIdentifierAuthority = (Value : (0,0,0,0,0,1)); SECURITY_WORLD_RID = $00000000; begin - // Windows - set security to allow non-elevated apps - // to access pipes created by elevated apps. - SIDAuthWorld := SECURITY_WORLD_SID_AUTHORITY; + sd := nil; everyone_sid := nil; - AllocateAndInitializeSid( SIDAuthWorld, 1, SECURITY_WORLD_RID, 0, 0, 0, 0, 0, 0, 0, everyone_sid); - - ZeroMemory( @ea, SizeOf(ea)); - ea.grfAccessPermissions := SPECIFIC_RIGHTS_ALL or STANDARD_RIGHTS_ALL; - ea.grfAccessMode := SET_ACCESS; - ea.grfInheritance := NO_INHERITANCE; - ea.Trustee.TrusteeForm := TRUSTEE_IS_SID; - ea.Trustee.TrusteeType := TRUSTEE_IS_WELL_KNOWN_GROUP; - ea.Trustee.ptstrName := PChar(everyone_sid); - - acl := nil; - SetEntriesInAcl( 1, @ea, nil, acl); + try + // Windows - set security to allow non-elevated apps + // to access pipes created by elevated apps. + SIDAuthWorld := SECURITY_WORLD_SID_AUTHORITY; + AllocateAndInitializeSid( SIDAuthWorld, 1, SECURITY_WORLD_RID, 0, 0, 0, 0, 0, 0, 0, everyone_sid); + + ZeroMemory( @ea, SizeOf(ea)); + ea.grfAccessPermissions := GENERIC_ALL; //SPECIFIC_RIGHTS_ALL or STANDARD_RIGHTS_ALL; + ea.grfAccessMode := SET_ACCESS; + ea.grfInheritance := NO_INHERITANCE; + ea.Trustee.TrusteeForm := TRUSTEE_IS_SID; + ea.Trustee.TrusteeType := TRUSTEE_IS_WELL_KNOWN_GROUP; + ea.Trustee.ptstrName := PChar(everyone_sid); + + acl := nil; + SetEntriesInAcl( 1, @ea, nil, acl); + + sd := PSECURITY_DESCRIPTOR( LocalAlloc( LPTR,SECURITY_DESCRIPTOR_MIN_LENGTH)); + Win32Check( InitializeSecurityDescriptor( sd, SECURITY_DESCRIPTOR_REVISION)); + Win32Check( SetSecurityDescriptorDacl( sd, TRUE, acl, FALSE)); + + sa.nLength := SizeOf(sa); + sa.lpSecurityDescriptor := sd; + sa.bInheritHandle := FALSE; + + // Create an instance of the named pipe + hPipe := Windows.CreateNamedPipe( PChar( FPipeName), // pipe name + PIPE_ACCESS_DUPLEX, // read/write access + PIPE_TYPE_MESSAGE or // message type pipe + PIPE_READMODE_MESSAGE, // message-read mode + FMaxConns, // max. instances + FBufSize, // output buffer size + FBufSize, // input buffer size + 0, // client time-out + @sa); // default security attribute + + FHandle := hPipe; + if( FHandle = INVALID_HANDLE_VALUE) + then raise TTransportException.Create( TTransportException.TExceptionType.NotOpen, + 'CreateNamedPipe() failed ' + IntToStr(GetLastError)); - sd := PSECURITY_DESCRIPTOR( LocalAlloc( LPTR,SECURITY_DESCRIPTOR_MIN_LENGTH)); - Win32Check( InitializeSecurityDescriptor( sd, SECURITY_DESCRIPTOR_REVISION)); - Win32Check( SetSecurityDescriptorDacl( sd, TRUE, acl, FALSE)); - - sa.nLength := SizeOf(sa); - sa.lpSecurityDescriptor := sd; - sa.bInheritHandle := FALSE; - - // Create an instance of the named pipe - hPipe := Windows.CreateNamedPipe( PChar( FPipeName), // pipe name - PIPE_ACCESS_DUPLEX, // read/write access - PIPE_TYPE_MESSAGE or // message type pipe - PIPE_READMODE_MESSAGE, // message-read mode - FMaxConns, // max. instances - FBufSize, // output buffer size - FBufSize, // input buffer size - 0, // client time-out - @sa); // default security attribute - - if( hPipe = INVALID_HANDLE_VALUE) then begin - FHandle := INVALID_HANDLE_VALUE; - raise TTransportException.Create( TTransportException.TExceptionType.NotOpen, - 'CreateNamedPipe() failed ' + IntToStr(GetLastError)); + finally + if sd <> nil then LocalFree( Cardinal( sd)); + if acl <> nil then LocalFree( Cardinal( acl)); + if everyone_sid <> nil then FreeSid(everyone_sid); end; - - FHandle := hPipe; - result := TRUE; end; diff --git a/lib/delphi/test/TestClient.pas b/lib/delphi/test/TestClient.pas index baeaaa8b..e72775ed 100644 --- a/lib/delphi/test/TestClient.pas +++ b/lib/delphi/test/TestClient.pas @@ -176,6 +176,10 @@ begin end else if (args[i] = '-anon') then // -anon begin + if Length(args) <= (i+2) then begin + Console.WriteLine('Invalid args: -anon or use "server.exe -anon"'); + Halt(1); + end; Console.WriteLine('Anonymous pipes transport'); Inc( i); hAnonRead := THandle( StrToIntDef( args[i], Integer(INVALID_HANDLE_VALUE))); -- 2.17.1