shared_from_this causing Exception tr1::bad_weak_ptr

I've been having a rotten week, with my boost::asio program keep giving me a segmentation fault... and it is not even doing the real work yet. It crashes when a client disconnects. The error message is:

   Exception: tr1::bad_weak_ptr

My code has now been littered with debug lines, lots of them showing usage counts of the shared_ptr in the hope of tracking down at what stage it is going wrong:

   std::cout << "this.use_count=" << shared_from_this().use_count() << "\n";

If that is unfamiliar, my class is defined like this:
   class client_session :
     public boost::enable_shared_from_this< client_session >{ ... }

This allows me to pass around shared pointers to this, from inside the class being pointed at, and is one of the essential tools you need to do anything useful with boost::asio.

I've been reading tutorials, studying other people's code, and progressively adding more shared pointers around objects that I am sure do not really need it. Nothing would shake it.

My code is using cross-references: the client connection object stores a vector of references to the data sources it uses, and each data source stores a vector of references to the clients subscribed to it. When I say reference I mean it holds a smart pointer instance. It is not that complicated but surely the problem must be in that cross-referencing? So, in desperation I deleted the entire data source class, and all that subscribing and unsubscribing code. Eh? It still crashes.

But then I noticed this code:
  ~client_session(){
    std::cout << "In client_session destructor (this.use_count="
        << shared_from_this().use_count() << ")\n";
    unsubscribe_from_all(); //'cos we'll no longer be valid after this
    }

I knew (!!) the problem was not in the destructor, but had to (!!) be before that point, because that first debug line was never reached. If you're already laughing at me, have a healthy helping of kudos. Yes, it was that call to shared_from_this() causing the crash! I was reaching the destructor, but crashing before it could print my debug line.

You see, in C++, an object does not really exist until the end of the constructor, and does not really exist when you enter the destructor. You must not use shared_from_this() in the destructor, or in any function called from the destructor. And when I thought again about what unsubscribe_from_all() (which was also calling shared_from_this()) does, I realized the destructor could not ever be called if any data sources still have a reference to us. So that call is not needed. The destructor code became:

  ~client_session(){
    std::cout << "In client_session constructor.\n";
    assert(subscriptions.size()==0);
    }

...and the crashes went away.

There is something very, very annoying knowing the bug I've been chasing for *two solid days* was in the debug code I added to track down the bug.

boost on centos (vs. on ubuntu)

You'd think porting a C++ program from one 64-bit linux to another would be trivial. But, no. A program developed with no issues on Ubuntu 10.04 was a lot of trouble to get to compile on Centos 5.6.
But get it to compile I did... read on for the secret words you have to utter...

First thing I did was:
   yum erase boost boost-devel

This got rid of the very old 1.33 library. I then ran this:

  yum install boost141 boost141-devel boost141-program-options boost141-regex boost141-thread boost141-system

I then had to hack the makefile to add this to my CFLAGS:

  -I/usr/include/boost141/

That got me compiling. But not linking. I changed my LDFLAGs to look like this but still it would not link:

  -L/usr/lib64/boost141 -lboost_regex -lboost_program_options -lboost_system -lboost_thread

I played around with various -L settings, until I had a breakthrough. I noticed the above line complained with:

  /usr/bin/ld: cannot find -lboost_thread

But some different -L settings instead gave me:

  /usr/bin/ld: cannot find -lboost_regex

Blink and you miss it, and indeed I had. My above line was linking with most of Boost, just not boost_thread. In other words, I was really close and hadn't realized it. A bit more poking around discovered that Ubuntu calls it "boost_thread", while Centos calls it "boost_thread-mt" (careful, the first one is an underline, the second one is a hyphen). So, the final magic line for Centos was:

  -L/usr/lib64/boost141 -lboost_regex -lboost_program_options -lboost_system -lboost_thread-mt

NOTE: I did not do any post-installation steps after the yum install steps. I saw some people suggesting copying files from one place to another but I did not need to. (Also, -L/usr/lib64 should be sufficient, as every thing is symlinked; maybe even the -L flag is not needed at all... but I'm in the "If it ain't broke." mindset now, so no more experimentation for me!)

C++ socket library: asio

It is when it comes to writing a simple socket client in C++ that I really feel how much PHP has spoilt me.

I previously have used SmartNetwork, part of the SmartWin library ( http://smartwin.sourceforge.net/ ) but it is fatally flawed: when the remote server dies there is no error reporting, so it happily carries on sending data to oblivion.

Therefore when an application I am currently working on needed to write to a socket I decided to try out a new library. I looked at 4 or 5 libraries, and after that first pass rejected all of them. Mainly based on their lack of clear documentation; sometimes based on their license. Faced with a choice of zero, I lowed my criteria, and chose to try boost::asio (also available in a non-boost version). Boost is a major project, already installed on my machines, portable for at least Linux/Windows, and the libraries are peer-reviewed by some very clever people. Boost libraries are also usually flexible (to the point of making them hard to use), and definitely able to give me the error reporting I need.

Here is the one line review: I've been banging my head against ASIO for 2-3 weeks, still do not have working code, but every time I consider running away I decide to stick with it.

Or in other words, it is terrible, but the alternatives are worse.

It is terrible in three ways: no high-level functions, it is hard to use properly and it is basically undocumented. Yeah, yeah, there is API documentation for all the functions, but it doesn't say when to use which function. And, yeah, yeah, there are a dozen or so tutorials. But they are all toy examples, and don't explain why the code has been written the seemingly-complex way it has.

Let me explain a bit more. Asio offers sync operations, but they are no use for any real-world code. For something you intend to use in production you will be forced to use the async functions. That means you'll need to use boost::threads, boost::bind (for the callbacks), boost::smart_pointer (as you have to wait for all callbacks to finish before you can delete an object, and it turns out your async callbacks can get called even after you've closed the socket) and understand how async programs work. All of that is hard.

I keep thinking I've got the code working, then I discover a timing problem that causes a crash only once in 15 runs, or it works on Linux but crashes on Windows, or is fine connecting to localhost but crashes when connecting to a remote server (due to different timing).

Going back to the first of my reasons for describing it as terrible, an example of the lack of high-level assistance is that there is no timeout parameter on any async operations. To do an async_connect that I want to give up after 3 seconds I have to write code for both the connect and the timer, which means two callbacks, and coordinating those two callbacks. Time-outs are part of the low-level BSD sockets but the asio code is doing something that deliberately cripples that, so trying to use them won't work either.

But I don't like to moan without being constructive. So I've been working on two things. First a high-level class called SocketFeeder (and SocketFeederSet) that you can just call write() on and not have to worry about all these callbacks. Second, a tutorial explaining how SocketFeeder has been written, and the thinking behind the design decisions.

The class is for a client's production environment but has been written on my own time, so I'll be able to release it as open source. I'll edit this blog to link to it when it is finally ready; if you are keen to see it and the tutorial then leave a comment or send me a mail. Constructive nagging usually works! (And if you want to sponsor me, or are a magazine that pays for articles, let me know; it will get released eventually, but financial incentive means I can give it priority.)

Escaping CSV in C++

There are two escaping rules for each field in a comma-separated value row:
1. Change each double quote to two double quotes.
2. Surround with double quotes if the field contains a comma or double quote.

These are the rules used by Excel and all other software that deals with CSV data.

As an example, if my fields are:
hello world
a,b,c
"CSV" is popular
""

Then it becomes:
hello world,"a,b,c,","""CSV"" is popular",""""""

C++ has a justified reputation as a hard language for text manipulation. Boost has libraries to make it a little easier, but I didn't want to add Boost as a dependency for a project I was working on. Fortunately std::string's replace() function turned out to be more powerful than I had realized:

void output_csv(std::ostream &out,std::string s){
if(s.find('"')!=std::string::npos){  //Escape double-quotes
    std::string::size_type pos=0;
    while(1){
        pos=s.find('"',pos);
        if(pos==std::string::npos)break;
        s.replace(pos,1,"\"\"");
        pos+=2; //Need to skip over those two quotes, to avoid an infinite loop!
        }
    out<<'"'<<s<<'"';
    }
else if(s.find(',')!=std::string::npos){ //Need to surround with "..."
    out<<'"'<<s<<'"';
    }
else out<<s;   //No escaping needed

If you like compact code then the while loop can be rewritten:

void output_csv(std::ostream &out,std::string s){
if(s.find('"')!=std::string::npos){  //Escape double-quotes
    for(std::string::size_type n=0;(n=s.find('"',n))!=std::string::npos;n+=2)s.replace(n,1,"\"\"");
    out<<'"'<<s<<'"';
    }
else if(s.find(',')!=std::string::npos)out<<'"'<<s<<'"';
else out<<s;
}

P.S. If you need to do the same in PHP, PHP 5.1 finally introduced fputcsv for it. The comments on that page show how to do it in older versions of PHP; my fclib library also contains functions for it.

Boost Spirit, shadowing, and a trailing newline

I've never been a fan of Boost::Spirit. It adds loads of compile time (in fact, some code failed to compile when I only had 512Mb, solely due to spirit), and the source code has shadowed variables (which causes complaints if you switch all warnings on, and therefore causes failure if you compile with -Werror).

I sent a patch in against 1.33 for the shadowed variable problems. So you can imagine my disappointment when I tried to compile against boost 1.34.1 (that comes with Ubuntu 7) and the problems were still there and required me to hack the source code again.
But that was nothing compared to how I felt when my unit tests failed to run. No compile errors, they simply failed to parse. My code was unchanged; only the boost version had changed.

I have finally tracked this down to if there is a trailing carriage-return it won't parse it. I thought the space_p parameter to parse() would take care of that, and perhaps that is the behaviour that changed between 1.33.1 and 1.34.1??

But most frustrating of all is that none of these solutions work:
'\n'
"\n"
ch_p('\n')
*ch_p('\n')
+ch_p('\n')
str_p("\n")
*str_p("\n")
+str_p("\n")

Actually I'm out of ideas!

Here is a code snippet:

GameTree = ch_p('(')[bind(&SGFMoveList::on_game_start, this, _1, fname)]
  >> RootNode[bind(&SGFMoveList::on_root_node_end, this, _1,_2)]
  >> *Node[bind(&SGFMoveList::on_node_end, this, _1,_2)] 
  >> *VariationTree 
  >> !ch_p(';') 
  >> ch_p(')')[bind(&SGFMoveList::on_game_end, this, _1)]
  >> *ch_p('\n');

parse_info<const char*> info=parse(str, *GameTree , space_p);

(I've left a ch_p('\n') in there at the end, just in case it is doing some good.)

So, my solution is to check info.hit instead of info.full! hit is true if the stuff you want matched; full is the same but only gets set if the whole input string got matched. In my case a carriage-return is not getting matched, so hit is true and full is false.

That is a bit crude and may be hiding a genuine problem. So I then have a look at info.stop (which is a pointer to the part of the input string that didn't get matched) to make sure it only contains whitespace.

If anyone knows what is going on please let me know. It amazes me that no-one else has noticed this regression in Spirit, so surely it must be something I'm doing wrong?

P.S. On the next project where I needed a parser I tried Hapy (http://hapy.sourceforge.net/) instead of Spirit. It was much faster and easier to use. Its downsides are practically no-one uses it, so not much documentation, community, etc.
On most projects since then I've used Boost::tokenizer wherever possible, going out of my way to avoid writing a real parser.
The above project, where I'm using Spirit, is complex, needs a real parser, and it works, hence my reluctance to port it to Hapy, or look for something else.