Re: Bug in common.cc initializing log_silencer_count_mutex_

2009-04-18 Thread Wink Saville
No problem at all. On Fri, Apr 17, 2009 at 6:24 PM, Kenton Varda wrote: > You know, after looking at it, I'm actually pretty uncomfortable with my > solution. I decided to use your original patch instead. Committed as rev > 110. Sorry for making you do the extra work. > > > On Thu, Apr 16, 20

Re: Bug in common.cc initializing log_silencer_count_mutex_

2009-04-17 Thread Kenton Varda
You know, after looking at it, I'm actually pretty uncomfortable with my solution. I decided to use your original patch instead. Committed as rev 110. Sorry for making you do the extra work. On Thu, Apr 16, 2009 at 10:22 PM, Wink Saville wrote: > Attached is a patch which changes Mutex to han

Re: Bug in common.cc initializing log_silencer_count_mutex_

2009-04-16 Thread Wink Saville
Attached is a patch which changes Mutex to handle the initialization ordering problem where Lock can be called before the constructor is called. On Wed, Apr 15, 2009 at 1:41 PM, Wink Saville wrote: > Fair enough on the Mutex, I'll try to get a new patch to you soon, > but if you get there first,

Re: Bug in common.cc initializing log_silencer_count_mutex_

2009-04-15 Thread Wink Saville
Fair enough on the Mutex, I'll try to get a new patch to you soon, but if you get there first, great. As for the explicit initialization I humbly disagree but I understand your point completely. Maybe someday there will be another initialization issue that isn't as easily solved and it can be rev

Re: Bug in common.cc initializing log_silencer_count_mutex_

2009-04-15 Thread Kenton Varda
On Tue, Apr 14, 2009 at 11:07 AM, Wink Saville wrote: > Feels strange to be using the object before it's constructed. Also, every > Mutex has the cost of the test. A predictable branch is basically free, especially compared to the cost of the atomic ops needed to actually lock a mutex. > If y

Re: Bug in common.cc initializing log_silencer_count_mutex_

2009-04-14 Thread Wink Saville
Feels strange to be using the object before it's constructed. Also, every Mutex has the cost of the test. If you want to go this way what about a "StaticMutex" which was implemented this way and leave the Mutex alone. I'm still in favor of an explicit initialization as a long term solution where a

Re: Bug in common.cc initializing log_silencer_count_mutex_

2009-04-14 Thread Kenton Varda
I think you could just do something like: enum LinkerInitialized { LINKER_INITIALIZED }; // Mutex allocated in static memory which may be locked before the // constructor is actually called. We can take advantage of the fact // that static memory is always initialized to zero. Mutex::Mutex(Linker

Re: Bug in common.cc initializing log_silencer_count_mutex_

2009-04-13 Thread Wink Saville
I did some searching and my initial reaction is that this requires some initialization of the mutex at load time which may not be possible in windows. But if you have a technique in mind let me know. I wonder if another option might be better in the long term. And that is to have an initialization

Re: Bug in common.cc initializing log_silencer_count_mutex_

2009-04-13 Thread Wink Saville
SGTM, I'll take a look when tonight. On Mon, Apr 13, 2009 at 5:26 PM, Kenton Varda wrote: > BTW, probably the ideal approach would be to make the Mutex class itself > immune to initialization ordering problems. That is, if you allocate a > Mutex statically, it shouldn't matter if its constructo

Re: Bug in common.cc initializing log_silencer_count_mutex_

2009-04-13 Thread Kenton Varda
BTW, probably the ideal approach would be to make the Mutex class itself immune to initialization ordering problems. That is, if you allocate a Mutex statically, it shouldn't matter if its constructor has been run yet when you try to lock it. (The Mutex class in our internal codebase works like t

Re: Bug in common.cc initializing log_silencer_count_mutex_

2009-04-13 Thread Kenton Varda
My backlog of patches and other work is very long, unfortunately. But I hope to spend some time reducing it this week, hopefully getting to this. On Sat, Apr 11, 2009 at 12:13 PM, Wink Saville wrote: > Has anyone been able to look at this, do I need to make some > changes or do anything else? >

Re: Bug in common.cc initializing log_silencer_count_mutex_

2009-04-11 Thread Wink Saville
Has anyone been able to look at this, do I need to make some changes or do anything else? On Wed, Apr 8, 2009 at 2:59 PM, Kenton Varda wrote: > Although I see the decsriptor.cc part isn't meant to be submitted... > > > On Wed, Apr 8, 2009 at 2:59 PM, Kenton Varda wrote: > >> You can use "svn d

Re: Bug in common.cc initializing log_silencer_count_mutex_

2009-04-08 Thread Wink Saville
Correct. Where you able to duplicate the problem. On Wed, Apr 8, 2009 at 2:59 PM, Kenton Varda wrote: > Although I see the decsriptor.cc part isn't meant to be submitted... > > > On Wed, Apr 8, 2009 at 2:59 PM, Kenton Varda wrote: > >> You can use "svn diff" to produce a single patch file that

Re: Bug in common.cc initializing log_silencer_count_mutex_

2009-04-08 Thread Kenton Varda
Although I see the decsriptor.cc part isn't meant to be submitted... On Wed, Apr 8, 2009 at 2:59 PM, Kenton Varda wrote: > You can use "svn diff" to produce a single patch file that contains both > parts of this change. > > > On Tue, Apr 7, 2009 at 10:01 PM, Wink Saville wrote: > >> There is a

Re: Bug in common.cc initializing log_silencer_count_mutex_

2009-04-08 Thread Kenton Varda
You can use "svn diff" to produce a single patch file that contains both parts of this change. On Tue, Apr 7, 2009 at 10:01 PM, Wink Saville wrote: > There is a bug in the initialization of the log_silencer_count_mutex_ in > common.cc. > Currently it is initialized because it is a global object,