Once I really had to implement something, it turned out that with about 10 lines of code in Thrift.h and Thrift.cpp I could avoid changing any occurrences of GlobalOutput. That's a small enough change that I think we can manage it in our local codebase.
Believe me, I would avoid this if I could. But I can't. At this point I have
picked apart all of the Thrift C++ code, and I know how I implemented the local
sockets, so I'm confident that I can know the data is initialized before I use
it. And once I put the visceral reaction aside and thought about it some more,
it's not like we're doing this during system startup, it's just at a point in
the client application's lifetime where you need to deal correctly with
indeterminate initialization order. Something we avoid like the plague, but it
doesn't quite require rocket science. Of course, maybe I just need to convince
myself that it will be okay, since I'm stuck with it. :-)
If GlobalOutput gets replaced with a real logging implementation, then we may
have issues. But the prerelease version I'm running on has all the features we
need to run our apps, so I've got choices.
Thank you for your thoughts,
Rush
On Sep 7, 2010, at 11:05 PM, Bruce Lowekamp wrote:
>
> I suppose that in the future, something like that could be extended so that a
> simple static-time logger that logs to stderr could be replaced with a
> non-static logger once the system is up, but that seems like a lot of
> complexity for something that I think isn't a good idea to do in the first
> place.
>
> Bruce
>
>
> On Sep 3, 2010, at 7:29 PM, David Reiss wrote:
>
>> 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
smime.p7s
Description: S/MIME cryptographic signature
