Trouble cleaning up after client goes down

Hi,

I have run into a strange problem. I am working on a server-client model (using the examples hclient.C and hserv2.C as a prototype), and I am trying to do some cleanup when a client goes down.

In my example, the server makes a copy of the histogram that the client sends. When it detects that the connection has been dropped, it deletes the (copy of the) histogram. When a new connection opens again, I’m getting a crash as soon as I attempt to read the first histogram from the new client.

I am attaching the modified macros that reproduce the problem. By setting the “use_my_own” flag to true, the server uses its own copy of the histogram (and the code crashes). If I set it to false, everything goes smoothly, no matter how many times the client goes down and then up again.

Any clues?

Cheers,

–Christos

PS $ which root
/afs/cern.ch/sw/root/v4.02.00/rh73_slc3/root/bin/root

The server macro:

{
  // if true, causes crashes when connections go down and then open again
  bool use_my_own = true;

   // Create canvas and pad to display the histogram
   TCanvas *c1 = new TCanvas("c1","The Ntuple canvas",200,10,600,480);
   c1->Draw();

   // Open a server socket looking for connections on a named service or
   // on a specified port.
   TServerSocket *ss = new TServerSocket(9090, kTRUE);

   TMonitor *mon = new TMonitor;
   mon->Add(ss);

   TSocket *s0 = 0;
   // this is my histogram
   TH1F * my_h = 0;
   // # of updates
   int updates = 0;

   while (1) {
     TMessage *mess;
     TSocket  *s = mon->Select();
     if (s->IsA() == TServerSocket::Class()) {
       if (!s0) {
	 s0 = ((TServerSocket *)s)->Accept();
	 s0->Send("go");
	 mon->Add(s0);
	 cout << " New connection opened " << endl;
       }
       else
	 printf("only accept one client connection\n");
       
       continue;
     }

     s->Recv(mess);

     if(!mess)
       {
	 cout << " *** Oops! Connection has been dropped " << endl;
	 // connection has been dropped - do some cleanup
	 mon->Remove(s0); s0->Close(); 
	 s0 = 0;
	 
	 if(use_my_own && my_h)	 
	   delete my_h;

	 continue;
       }

     ++updates;
     cout << " # of updates = " << updates << endl;

     if (mess->What() == kMESS_OBJECT) {
       TH1F *h = (TH1F *)mess->ReadObject(mess->GetClass());
       
       if(use_my_own && !my_h)
	 {
	   my_h = new TH1F("my name", h->GetTitle(), h->GetNbinsX(), 
			   h->GetXaxis()->GetXmin(), 
			   h->GetXaxis()->GetXmax());
	   my_h->SetDirectory(0);
	 }

       if(use_my_own)
	 {
	   h->Copy(*my_h);
	   my_h->Draw();
	 }
       else
	 h->DrawCopy();  

       c1->Modified();
       c1->Update();

       delete h;       // delete histogram that has been read in
              
     }
     else {
       printf("*** Unexpected message ***\n");
     }

     delete mess;
   }


   // we get here only if we break out of the loop
   s0->Close();   // close the socket
   
}

The client macro:

{
   // Open connection to server
   TSocket *sock = new TSocket("localhost", 9090);

   // Wait till we get the start message
   char str[32];
   sock->Recv(str, 32);

   TH1 *hpx;
   // Create the histogram
   hpx = new TH1F("hpx","This is the px distribution",100,-4,4);
   hpx->SetFillColor(48);  // set nice fillcolor
   hpx->SetFillStyle(0);

   TMessage mess(kMESS_OBJECT);

   // Fill histogram randomly
   gRandom->SetSeed();
   Float_t px;
   bool stay_in_loop = true;
   while(stay_in_loop)
     {
       for(int i = 0; i != 100; ++i)
	 {
	   px = gRandom->Gaus(0, 1);
	   hpx->Fill(px);
	 }
       
       mess.Reset();              // re-use TMessage object
       mess.WriteObject(hpx);     // write object in message buffer
       sock->Send(mess);          // send message
       
       gSystem->Sleep(1000); // wait for 1 sec

     } // infinite loop

   // we get here only if we break out of the loop
   sock->Close();    // close the socket

}

Dear Christos,

You must reset to 0 the pointer after deleting the object:

if (use_my_own && my_h) {
delete my_h;
my_h = 0;
}

otherwise at the next opening you try to re-use an object which does not exists anymore.

Gerri Ganis

Hi Ganis,

I see your point, but your suggestion does not really solve the problem, only delays the crash. :slight_smile:

First of all, the command “h->Copy(*my_h)” does not just copy the contents of h into my_h, but also changes the name of my_h (from “my name” to “hpx”). This may be the root of my problems.

When I introduce your “my_h = 0” suggestion, and I try to stop & restart the client program a couple of times, I get the following:

(a) it works for a couple of times
(b) it may give me a warning for a potential memory leak:

Warning in <TH1::Build>: Replacing existing histogram: myname (Potential memory leak).

This inconsistent warning confuses the heck out of me. If I lose track of “my_name” (hence the warning), then how can I invoke the TH1F::Copy command without losing track of the original name? And, why does it occasionally work (suggesting that my_name was really deleted)?

If I create my_h with the same name (ie. “hpx”), I always get a warning for a potential memory leak (which I understand).

© I will eventually crash [whether (b) happened or not]

I’m trying to do something very simple:

  • extract a temporary object from a TMessage
  • copy it somewhere so I can access it later
  • delete the temporary object.

Maybe I shouldn’t be using TH1F::Copy but something else? I could even do a loop over the hpx bins and manually copy the bin contents (but I’m sure there’s a better solution).

Thanks for your time!

Hi Christos,
h->Copy(*my_h) also replaces my_h’s name and my_h’s fDirectory. So using TH1F* my_h=(TH1F*)h->Clone("newname"); my_h->SetDirectory(0); might help. There was a problem with deleting copied histograms which is fixed in cvs. But if at all possible I’d try to prevent the hist from being copied, and just drag several pointers to the same hist around.
Axel.

Here’s the catch: there’s more than one “entities” in my code that hold on to that histogram address (think of “how many clients are interested in that histogram”). So, by copying the “h” contents into a location speficied by a (fixed) TH1F * pointer (“my_h”), I take care of everything. If I allow for the “my_h” guy to point to different locations, I would have to update the pointers in all those classes. Yack.

It sounds like doing it manually

// Loop over bins; index: i
my_h->SetBinContent(i, h->GetBinContent(i));
my_h->SetBinError(i, h->GetBinError(i));

is the only solution? 8-[

Is there a “magic” TH1 method that does just that? :wink:

Hi Christos,
why don’t you simply store a TH1* & in the clients, pointing to TH1* h, as in TH1* &myref = h; (or TH1** myref = &h; if you don’t like references). That way you can get new histogram addresses in h, and all the clients will still know how to find the histos (as they go myref->h->histogram).

Other than that calling h->SetDirectory(0) before copying it and then renaming the copied histogram might help. Again - this has been fixed in cvs as far as I know.
Axel.

Hi Christos,

Indeed my fix was working with the CVS head but not with 4.02.00: this must be related to the problem mentioned by Axel.

Back to what you would like to have, it seems to me that this construct may fit your needs:

       TH1F *h = (TH1F *)mess->ReadObject(mess->GetClass());

       // Create reference histo object the first time 
       if(!my_h) {
           my_h = new TH1F("my name", h->GetTitle(), h->GetNbinsX(),
           h->GetXaxis()->GetXmin(),
           h->GetXaxis()->GetXmax());
           my_h->SetDirectory(0);
       }

       // Reset reference histo
       my_h->Reset();
       // Add the one read in
       my_h->Add(h);
       // Draw
       my_h->Draw();

       c1->Modified();
       c1->Update();

       delete h;       // delete histogram that has been read in

In this way my_h is always the same pointer and gets filled each time with the new histo. This seems to work fine also with 4.02.00 .

Cheers, Gerri

Hi again,

Thanks to both of you for your suggestions. Of course, they both worked.

I’m especially impressed by gani’s “dirty” workaround: :slight_smile:

// Reset reference histo
my_h->Reset();
// Add the one read in
my_h->Add(h); 

That did the trick for me!

Axel’s suggestion may be more elegant, but I really want to delete “h” after the TObject extraction because I use it to read in a ton of histograms in every client-server transaction cycle.

Thanks for the weekend support. :slight_smile:

–Christos