Sounds good. Feel free to submit the patch. Let's just consider this an
undocumented feature of Thrift, rather than a part of the interface we're
committed to keep supporting.
--David
On 09/08/2010 09:22 AM, Rush Manbert wrote:
> 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
>
