Print

---

Hi Chris,

  I know them all, and will put my eye on them in a short time. Anyway 
it's alpha code and I am revisiting it all in these days.

  BTW, this remark:

 > Code:
 >     XrdClientPhyConnection::BuildMessage returns a new XrdClientMessage,
 > but in SocketReaderThread we see
 >
 >    while (1) {
 >      thisObj->BuildMessage(TRUE, TRUE);
 >      thisObj->CheckAutoTerm();
 >    }
 >     the return value is ignored
 >
 > Resolution:
 >     -delete the value returned from the line '
 > thisObj->BuildMessage(TRUE, TRUE);'
 >     -better solution is to have XrdClientPhyConnection::BuildMessage
 > return a
 >         std::auto_ptr<XrdClientMessage>

is not correct, since the BuildMessage parameters tell it to enqueue 
what it created. So it's not a good idea to delete a message that has 
been enqueued somewhere else.
That leak is related to a condition for the message deletion inside 
SendGenCommand/ClientServerCmd. It does not loose all the messages. Only 
one in a particular situation. This does not mean that it won't be fixed.

Fabrizio


Chris Jones wrote:
> I've run XrdClient code through the Valgrind memory checker and have 
> found multiple leaks.
> 
> 1-------------------------------------------------------------
> Valgrind:
> ==27481== 32 bytes in 1 blocks are definitely lost in loss record 5 of 22
> ==27481==    at 0x1B903E96: operator new(unsigned) 
> (vg_replace_malloc.c:133)
> ==27481==    by 0x8064780: XrdClientConn::DoHandShake(short) 
> (XrdClientConn.cc:1022)
> ==27481==    by 0x8062BD4: XrdClientConn::GetAccessToSrv() 
> (XrdClientConn.cc:804)
> ==27481==    by 0x805BA80: XrdClient::Open(unsigned short, unsigned 
> short) (XrdClient.cc:160)
> 
> Code:
>             fLBSUrl = new XrdClientUrlInfo(fUrl.GetUrl());
> 
> Resolution:
>     -The destructor ~XrdClientConn must delete 'fLSBUrl' AND before each 
> 'fLBSUrl = new ...' must do a delete
>     -A better solution would be to make fLSBUrl a 
> std::auto_ptr<XrdClientUrlInfo>
> 
> 2-------------------------------------------------------------
> Valgrind:
> ==27481== 80 bytes in 4 blocks are definitely lost in loss record 9 of 22
> ==27481==    at 0x1B903E96: operator new(unsigned) 
> (vg_replace_malloc.c:133)
> ==27481==    by 0x8070934: XrdClientPhyConnection::BuildMessage(bool, 
> bool) (XrdClientPhyConnection.cc:384)
> ==27481==    by 0x806EBA3: SocketReaderThread 
> (XrdClientPhyConnection.cc:50)
> 
> Code:
>     XrdClientPhyConnection::BuildMessage returns a new XrdClientMessage, 
> but in SocketReaderThread we see
> 
>    while (1) {
>      thisObj->BuildMessage(TRUE, TRUE);
>      thisObj->CheckAutoTerm();
>    }
>     the return value is ignored
> 
> Resolution:
>     -delete the value returned from the line ' 
> thisObj->BuildMessage(TRUE, TRUE);'
>     -better solution is to have XrdClientPhyConnection::BuildMessage 
> return a
>         std::auto_ptr<XrdClientMessage>
> 
> 3-------------------------------------------------------------
> Valgrind:
> ==27481== 231 bytes in 9 blocks are definitely lost in loss record 15 of 22
> ==27481==    at 0x1B903D1C: malloc (vg_replace_malloc.c:131)
> ==27481==    by 0x4207A5EF: strdup (in /lib/tls/libc-2.3.2.so)
> ==27481==    by 0x807FF7B: XrdNetDNS::getHostName(sockaddr&, char**, 
> int, char**) (in /A/lnx106/cdat/linux/t
> em/cdj/build/xrootd/Linux/bin/testExe.exe)
> ==27481==    by 0x807FE01: XrdNetDNS::getHostName(sockaddr&, char**) (in 
> /A/lnx106/cdat/linux/tem/cdj/build/
> xrootd/Linux/bin/testExe.exe)
> ==27481==    by 0x807FDC9: XrdNetDNS::getHostName(char*, char**) (in 
> /A/lnx106/cdat/linux/tem/cdj/build/xroo
> td/Linux/bin/testExe.exe)
> ==27481==    by 0x8067894: XrdClientConn::GetDomainToMatch(std::string) 
> (XrdClientConn.cc:1545)
> ==27481==    by 0x805F88F: XrdClientConn::XrdClientConn() 
> (XrdClientConn.cc:74)
> ==27481==    by 0x805AB5E: XrdClient::XrdClient(char const*) 
> (XrdClient.cc:50)
> 
> Code:
>     The 2nd argument of XrdNetDNS::getHostName(sockaddr&, char**, int, 
> char**) returns a new 'char*' which is not deleted.  This value is 
> returned from XrdNetDNS::getHostName(char*, char**)
> 
> 
> Resolution:
>      -XrdClientConn::GetDomainToMatch(..) should delete the value 
> returned from XrdNetDNS::getHostName
>      -better, change the code to use 'std::string' instead of 'char*'
> 
> 
>     Chris