I'd be fine with this change, but I also think that it would be a good idea
for Thrift to switch to a real logging library at some point. When that time
comes, I don't want the choice to be limited to libraries that can safely be
used during static initialization. Seriously, "don't do that."
--David
On 09/03/2010 04:46 PM, Rush Manbert wrote:
> I'm hoping the hard core C++ guys will think about this a little and maybe
> have some suggestions. It's an esoteric enough problem that you might find it
> interesting. :-)
>
> For complicated reasons, we sometimes end up trying to open a Thrift socket
> connection to a local server process during client-side static init time. I
> know that the simple answer would be "don't do that", but that's really not
> possible.
>
> This has worked just fine as long as the server process is running. If it is
> not running, however, the client app has a high probability of crashing when
> the socket connection attempt fails. This is because when such an error
> occurs, the output is handled by calling GlobalOutput.perror, like so:
>
> GlobalOutput.perror("TSocket::open() socket() " + getSocketInfo(),
> errno_copy);
>
> The problem is that GlobalOutput is a global object that is constructed at
> static init time. Since we can't control the order in which global objects
> are initialized, we may try to communicate before GlobalOutput has been
> constructed. Thus the crash. (The situation is analogous to a global object
> trying to connect to the server in the object's constructor. Depending on the
> order of static init execution, GlobalOutput may or may not have been
> constructed when the connection attempt is made.)
>
> The GlobalOutput object is instantiated in Thrift.cpp with this line of code:
>
> TOutput GlobalOutput;
>
> The easy way to fix this problem is to wrap the global object in a "construct
> on first use" function
> (http://www.parashift.com/c++-faq-lite/ctors.html#faq-10.15) like this:
>
> TOutput &GlobalOutput (void)
> {
> static TOutput GlobalOutput;
> return GlobalOutput;
> }
>
> which guarantees that it gets constructed the first time GlobalOuput() is
> called, but then I will need to change every existing reference to
> GlobalOutput to a function call to GlobalOutput(). So, for instance, the call
> to perror that I pasted above would change to:
>
> GlobalOutput().perror("TSocket::open() socket() " + getSocketInfo(),
> errno_copy);
>
>
> As usual, I have a list of questions (but it's short):
>
> 1) Can anyone think of another way around this problem?
>
> 2) Would the C++ library developers be at all receptive to taking the change
> I have outlined (GlobalOutput -> GlobalOutput()) into the trunk? On one hand,
> it adds a function call. On the other hand the operation in progress is
> formatted output, so one extra function call to get the reference probably
> doesn't matter. On yet the other hand, the change would have to extend to the
> server side code as well, and probably you guys running large server side
> operations worry about error performance more that I worry about client side
> performance in the same situation.
>
> TIA,
> Rush
