Comment On Race and Government

...bait

wow that's nice, so this problem obviously is supposed to make the clerks ready for multitasking :-)
At least this would make it easier to annoy people simultaniously

The cs is local? Nice. That's a not-so-critical section then.

BTW, you should see how these "wait in line" ticket systems work here in my country.

Not only is the critical section local, the code also doesn't call InitializeCriticalSection.

But if you're going to shoot yourself in the foot, better to do both feet, I guess.

1) The cs variable is declared local? Very considerate to give every thread it's own private critical section. :-)

2) Shouldn't a critical section be initialized with InitializeCriticalSection() before use?

Regarding the waiting lines; still better then a large LED display with only the last ticket-teller combo. Try to figure out where to go when two or more are pushing their free-button almost simultaneous.

Are you sure that it wasn't re-announcing on purpose? It could be that someone didn't appear at the station they were supposed to, so they called the number again. I've seen that happen at the DMV, even without the concurrent calls :-)

I worked on some C++ code where the locking code consisted of declaring a reference to a local variable. As in

ThreadLock &a;

That was it. I thought "hey, that code doesn't do anything", but when I removed it the system would fail. It took me a long time to figure out that the constructor of that particular class grabbed a global mutex lock, and the destructor released it. I'm sure the author of the code thought it was very clever, but to me that's too close to a "jedi mind trick". If it had been up to me, it would have been

ThreadLock &a;
a.grabLock();
...
a.releaseLock();

Sure, have the destructor release it if you still have it, but at least it's clearer to those who come after you what you're attempting to do.

ptomblin:

I'm sure the author of the code thought it was very clever, but to me that's too close to a "jedi mind trick".

I get your point; however, don't you agree that the automatic lock grab/release is much higher level?

At least your drivers liscense station had such a system. My local one consisted of:

"OK. Whose next? What's your number?"
"X"
"Wait, does anybody have a number lower than X?"

There were only two of us there. (I got lucky.)

I'm guessing the DMV considered this bug "non critical" har har, I kill me.

ptomblin:

I worked on some C++ code where the locking code consisted of declaring a reference to a local variable. As in

ThreadLock &a;

That was it. I thought "hey, that code doesn't do anything", but when I removed it the system would fail. It took me a long time to figure out that the constructor of that particular class grabbed a global mutex lock, and the destructor released it. I'm sure the author of the code thought it was very clever, but to me that's too close to a "jedi mind trick". If it had been up to me, it would have been

ThreadLock &a;
a.grabLock();
...
a.releaseLock();

Sure, have the destructor release it if you still have it, but at least it's clearer to those who come after you what you're attempting to do.

The problem with your solution is that it breaks down with exceptions. If an exception is thrown between grabLock and releaseLock, you're hosed. Or, you have to manually add a try/catch block and handle every possible exception manually, release the lock, and rethrow. Personally, I use the ACE libs for a lot of my threading the following works great:

static ACE_Mutex lock;
ACE_Guard<ACE_Mutex> guard(lock);
// do something useful

Don't knock something just because you don't understand it. Take the time to learn about it, then knock it.

That's fine C++ code. It follows the mantra that "resource acquisition is initialization". Initialization of the ThreadLock object acquires the mutex.

Regardless of whether you like it, it's such a standard idiom in C++ that any practicing programmer needs to be familiar with it, because they're bound to run into examples.

H|B:

ptomblin:

I'm sure the author of the code thought it was very clever, but to me that's too close to a "jedi mind trick".

I get your point; however, don't you agree that the automatic lock grab/release is much higher level?

uhmmm... just let me say RAII!!!!

diaphanein:

ptomblin:

-SNIP-

The problem with your solution is that it breaks down with exceptions. If an exception is thrown between grabLock and releaseLock, you're hosed. Or, you have to manually add a try/catch block and handle every possible exception manually, release the lock, and rethrow. Personally, I use the ACE libs for a lot of my threading the following works great:

static ACE_Mutex lock;
ACE_Guard<ACE_Mutex> guard(lock);
// do something useful

Don't knock something just because you don't understand it. Take the time to learn about it, then knock it.

He did say that he'd have the destructor release the lock if it hadn't been done manually already.

dave:

That's fine C++ code. It follows the mantra that "resource acquisition is initialization". Initialization of the ThreadLock object acquires the mutex.

Regardless of whether you like it, it's such a standard idiom in C++ that any practicing programmer needs to be familiar with it, because they're bound to run into examples.

Well, fine except for the uninitialized reference. ;)

Additionally, you should always like something that makes your job easier and your code safer.

http://en.wikipedia.org/wiki/Resource_Acquisition_Is_Initialization

I worked on some C++ code where the locking code consisted of declaring a reference to a local variable. As in

ThreadLock &a;

First off, as you've written it, no constructor will be called, since it's just declaring a reference but not initializing it. But if you intended to write ThreadLock(&a), that's still painful because the pointer to the local is totally unnecessary as any needed object to distinguish the mutex could be encapsulated in ThreadLock itself, and because the scope of the anonymous ThreadLock is implementation-dependent so the constructor might be called immediately instead of at the end of the block. So maybe you intended to write "ThreadLock a", which could perfectly well work, but then I'm left wondering if you actually don't know the difference between an object and a reference.

Care to enlighten us?

BTW, this idiom is common enough practice in C++ to have its own name and acronym: RAII, resource acquisition is initialization. Personally, I like to use a naming convention for such classes in order to make it immediately obvious that the class does its work in ctor & dtor, but if you're going to work in C++ you'd better know about this technique.

At our local DMV in my state, you're treated to the spectacle of the employees slowly shuffling off to the back, or to a water fountain, after each customer.

They shuffle around like zombies or something... they are certainly in no hurry to get to the next person.

They could save money (by hiring fewer workers) if they would give each employee some reasonable incentive, like 50 cents or $1.00, for each (correctly) completed transaction.

..it's a floor wax AND a desert topping. RAII is keen and it's a jedi mind trick. To help out my future readers, all of my RAII-type class start with the letters 'RAII' and end with the name of the function to be called.

As in: RAIIfclose to auto-fclose a file. RAIIRelease to release an IUnknown.

Or is good advice banned here?

diaphanein:

dave:

That's fine C++ code. It follows the mantra that "resource acquisition is initialization". Initialization of the ThreadLock object acquires the mutex.
...

Well, fine except for the uninitialized reference. ;)

C++ assigns the value T() to any non-initialized variable of type T. (That is why not defining a constructor with no parameters leads to trouble.) So, it is initialized, you just don't see that.

Peter:

..it's a floor wax AND a desert topping. RAII is keen and it's a jedi mind trick. To help out my future readers, all of my RAII-type class start with the letters 'RAII' and end with the name of the function to be called.

As in: RAIIfclose to auto-fclose a file. RAIIRelease to release an IUnknown.

Or is good advice banned here?

I wouldn't call it banned, so much as terribly uncommon. :)

Oh, bah humbug. It's still early and I'm having caffeine withdrawal. C++ does not allow uninitialized references. ThreadLock &a should not compile, but I suppose it's possible that some non-compliant compiler might allocate a temporary and initialize the reference to it, but what exactly would the lifetime of that object be? A reference going out of scope does not (normally) call the destructor, and in fact that behavior would be very broken. But of course the other alternative is not much better, not calling the destructor wouldn't release the lock. Perhaps a bizarre special case for a reference initialized to an anonymous instance? That also would violate the standard, but... Hmm.

Perhaps the poster meant: ThreadLock &a = ThreadLock(), which would still be broken because the lifetime of the still anonymous ThreadLock object is still implementation-dependent. Sheesh. C++ programmers who don't understand what this does:

ThreadLock a;

and so go off thrashing with:

ThreadLock &a = ThreadLock();

ooh, while you were at it, why didn't you use placement-new syntax too:

ThreadLock &a = *new ThreadLock(malloc(sizeof(ThreadLock));

Geezuz people, READ A BOOK once in a while, you might learn something! Broken code, critiqued by a submitter who doesn't know what a reference is. A double WTF!! Now, let's watch the comments and see if we can make it a triple or better ;-)

My best DMV WTF:

I got a license renewal notice last year- addressed correctly. I filled it in, and didn't check the 'address changed box', because my address didn't change. Of course I also didn't fill in the new address area, because my address didn't change. 2-6 weeks later, I got my new license in the mail. The envelope was addressed correctly. The WTF is that the address listed on my license (which should be the same as my mailing address- I only ever gave them one address) is off by one digit in the zip code. A 1 is replaced with a 7.

I can just imagine the process by which they re-enter your address to be printed on the license by hand.

C++ assigns the value T() to any non-initialized variable of type T. (That is why not defining a constructor with no parameters leads to trouble.) So, it is initialized, you just don't see that.

Yes, but C++ does NOT assign T() to a non-initialized variable of type T&! At least not standards-compliant C++, and any compiler that did so would itself be a huge WTF, because of the question of the object lifetime!

danielpitts:

Are you sure that it wasn't re-announcing on purpose? It could be that someone didn't appear at the station they were supposed to, so they called the number again.

Good call, but I watched it happen for 15 minutes, every time. What I observed was: single announcements never had duplicates; multiple announcements always had duplicates; it was always the first announcement that was duplicated --- and no others --- being announced (for the second time) last.

I could almost imagine that the clerks were pushing the buttons multiple times except that the first clerk to push the button had instant feedback AND their assignment showed up twice in a row on the monitors, not in the order of the announcements.

Kid:

At least your drivers liscense station had such a system. My local one consisted of:

"OK. Whose next? What's your number?"
"X"
"Wait, does anybody have a number lower than X?"

There were only two of us there. (I got lucky.)

You know, it's almost worthwhile to wait until their counter cycles back to 1, to see if they would actually say "Does anybody have a number lower than 1?". Even better, would the person with 99 (assuming a roll of 99 number tickets) realize that, in this context, 99 is less than 1?

In Soviet Russia, ticket line works you!

nobody:

C++ assigns the value T() to any non-initialized variable of type T. (That is why not defining a constructor with no parameters leads to trouble.) So, it is initialized, you just don't see that.

Yes, but C++ does NOT assign T() to a non-initialized variable of type T&! At least not standards-compliant C++, and any compiler that did so would itself be a huge WTF, because of the question of the object lifetime!

Exactly. That's why I said uninitialized reference.

      1 struct Foo

      2 {};

      3

      4 int main(int argc, const char* argv[])

      5 {

      6     Foo &f;

      7

      8     return 0;

      9 }

     10

Fails to compile on gcc 4.1:

main.C: In function `int main(int, const char**)':
main.C:6: `f' declared as reference but not initialized

Mike:

In Soviet Russia, ticket line works you!

Kinky.

What's a line? All I see when doing any burocratic proceedure are curves

CAPTCHA: Me "digdug" thru those curves, cause I don't like ping-pong... man this wednesday afternoon it's hitting hard on me.

so a simple solution would be to separate the input and output:
Step 1: Clerk pushes button, signal is sent into a message queue with timestamp and a unique message identifier
Step 2: a process picks up the messages in the queue every n seconds and the audio messages are played (spearated by a pause of x seconds). Each audio message played corresponds with it's designator coming up on the message board

if two (or more) messages arrive at the exact same time, they go in random order.

Meanwhile, on the other side of the country...

At New York City's Penn Station, a slow-talking nasal voice ('terror lady') comes on every x minutes to warn us with something along the lines of "New Jersey Transit and Amtrak request that all passengers stay alert and aware of their surroundings at all times. Please report any suspicious packages or activity to Amtrak police ..." etc. etc. etc.

Now when the stationmaster needs to announce a departure or track assignment -- which as you can imagine happens pretty much constantly at Penn Station during rush hour -- it interrupts the terror warning.

Once the (proper) announcement is done, terror lady picks up again -- from the very beginning. Then another track is announcement is made, and when that's done terror lady starts one more time. From the very beginning.

By the time she finally makes it to the end of her spiel, the x minutes are up. And it's time for her to start again.

Mike:

In Soviet Russia, ticket line works you!

Speaking of references -- that's gotta be the weakest Smirnoff reference ever.

Perhaps the poster meant: ThreadLock &a = ThreadLock(), which would still be broken because the lifetime of the still anonymous ThreadLock object is still implementation-dependent.

No, it isn't. If a reference is initialized with a temporary, the lifetime of the temporary is extended to the lifetime of this reference. There's nothing implementation-dependent about it.

My favorite:
In Soviet Russia, the government controls commerce.

CornedBee:

Perhaps the poster meant: ThreadLock &a = ThreadLock(), which would still be broken because the lifetime of the still anonymous ThreadLock object is still implementation-dependent.

No, it isn't. If a reference is initialized with a temporary, the lifetime of the temporary is extended to the lifetime of this reference. There's nothing implementation-dependent about it.

But I believe in this context it'd have to be a "const ThreadLock &a = ThreadLock ()", which is only useful if you don't need to release/acquire during its lifetime. I can't think of a definition of ThreadLock for which "ThreadLock &a = ThreadLock ()" will compile.

We need a version of Godwin's Law that applies to C++ debates like the one raging above. Maybe "Whoever produces GCC output supporting their side wins summarily unless the opponent can produce conflicting compiler output within 10 posts"?

Seriously: declare an instance, the constructor is called. Call the "new" operator explicitly, the constructor is called. Declare a pointer, the constructor is not called. And as has been demonstrated above, you can't declare a naked reference. The debate has ended.

As for whether or not RAII is a good idea -- if it's not in Stroustrup's book, it's a trick. This does *not* mean you can't use it, or that using it is a WTF, but it does merit at least a few words of comment. Is:

{
...
Threadlock a; // Constructor locks the mutex
...
// Threadlock destructor will release mutex
}

so hard to type?

nobody:

A reference going out of scope does not (normally) call the destructor,

Yeah, maybe you should read up on temporary objects

nobody:

Perhaps the poster meant: ThreadLock &a = ThreadLock(), which would still be broken because the lifetime of the still anonymous ThreadLock object is still implementation-dependent.

This will not compile, it's illegal to assign an r-value to a reference to non-const type. But in case of "const Threadlock &a = ThreadLock();", the lifetime of the temporary is clearly defined, it's the same as that of a. It is not implementation-defined (or "implementation-dependent", as you call it).

nobody:

ooh, while you were at it, why didn't you use placement-new syntax too:

ThreadLock &a = *new ThreadLock(malloc(sizeof(ThreadLock));

The correct placement-new syntax is, of course

ThreadLock &a = *new (malloc(sizeof(ThreadLock)))

	ThreadLock();

and you also have to provide a (global or class static)

#include <cstddef>



void *operator new(std::size_t, void *ptr)

{

	return ptr;

}

nobody:

Geezuz people, READ A BOOK once in a while, you might learn something!

Indeed...

Uberbandit:

What's a line? All I see when doing any burocratic proceedure are curves

Would a "burocratic" procedure involve having donkeys fill out the paperwork, or just approve it. Either way, it would speed things up in the DMV.

But in case of "const Threadlock &a = ThreadLock();", the lifetime of the temporary is clearly defined, it's the same as that of a. It is not implementation-defined (or "implementation-dependent", as you call it).

Right. I was confusing that with the case of a bare temporary not bound to anything:

MyClass();

Pretty pointless thing to do, usually. But legal, and not the hardest typo to make when tired, and something that I actually did once. It worked in the compiler I was using at the time, but after moving to a different compiler introduced a nasty race. I about smashed my head through my desk when I finally found the problem ;-)

The DMV issue has really nothing to do with any computer code. There are simply DMV station operators who are in the habit of pushing their call button twice. If the OP had stuck around, he would have seen that #23 is one of those operators, regardless of whether another announcement is made over top of his first one or not. Sorry to ruin a good story.

AdT:

nobody:

A reference going out of scope does not (normally) call the destructor,

Yeah, maybe you should read up on temporary objects

nobody:

Perhaps the poster meant: ThreadLock &a = ThreadLock(), which would still be broken because the lifetime of the still anonymous ThreadLock object is still implementation-dependent.

This will not compile, it's illegal to assign an r-value to a reference to non-const type. But in case of "const Threadlock &a = ThreadLock();", the lifetime of the temporary is clearly defined, it's the same as that of a. It is not implementation-defined (or "implementation-dependent", as you call it).

nobody:

ooh, while you were at it, why didn't you use placement-new syntax too:

ThreadLock &a = *new ThreadLock(malloc(sizeof(ThreadLock));

The correct placement-new syntax is, of course

ThreadLock &a = *new (malloc(sizeof(ThreadLock)))

	ThreadLock();

and you also have to provide a (global or class static)

#include <cstddef>



void *operator new(std::size_t, void *ptr)

{

	return ptr;

}

nobody:

Geezuz people, READ A BOOK once in a while, you might learn something!

Indeed...

I'm to bored to figure out who started this D@$$ measuring contest, but think you need look at your source a little closer.

PseudoNoise:

CornedBee:

Perhaps the poster meant: ThreadLock &a = ThreadLock(), which would still be broken because the lifetime of the still anonymous ThreadLock object is still implementation-dependent.

No, it isn't. If a reference is initialized with a temporary, the lifetime of the temporary is extended to the lifetime of this reference. There's nothing implementation-dependent about it.

But I believe in this context it'd have to be a "const ThreadLock &a = ThreadLock ()", which is only useful if you don't need to release/acquire during its lifetime. I can't think of a definition of ThreadLock for which "ThreadLock &a = ThreadLock ()" will compile.

There is none. The temporary ThreadLock is an rvalue, which cannot be converted to a non-const reference. GCC will give you an error like:

initialization of non-const reference type `struct Foo&' from rvalue
of type `Foo'

What's also fun is getting code to compile when you try and return an object by value which takes a non-const reference in its copy constructor (e.g. auto_ptr).

      1 struct Foo

      2 {

      3     Foo()

      4     {}

      5

      6     Foo(Foo&)

      7     {}

      8

      9 //* Comment out the following block to break line 26

     10     struct ReturnHelper

     11     {

     12         ReturnHelper(Foo&)

     13         {}

     14     };

     15

     16     Foo(ReturnHelper)

     17     {}

     18

     19     operator ReturnHelper()

     20     { return ReturnHelper(*this); }

     21 //*/

     22 };

     23

     24 int main(int argc, const char* argv[])

     25 {

     26     Foo f = Foo();

     27

     28     return 0;

     29 }

     30

no name:

AdT:

nobody:

A reference going out of scope does not (normally) call the destructor,

Yeah, maybe you should read up on temporary objects

nobody:

Perhaps the poster meant: ThreadLock &a = ThreadLock(), which would still be broken because the lifetime of the still anonymous ThreadLock object is still implementation-dependent.

This will not compile, it's illegal to assign an r-value to a reference to non-const type. But in case of "const Threadlock &a = ThreadLock();", the lifetime of the temporary is clearly defined, it's the same as that of a. It is not implementation-defined (or "implementation-dependent", as you call it).

nobody:

ooh, while you were at it, why didn't you use placement-new syntax too:

ThreadLock &a = *new ThreadLock(malloc(sizeof(ThreadLock));

The correct placement-new syntax is, of course

ThreadLock &a = *new (malloc(sizeof(ThreadLock)))

	ThreadLock();

and you also have to provide a (global or class static)

#include <cstddef>



void *operator new(std::size_t, void *ptr)

{

	return ptr;

}

nobody:

Geezuz people, READ A BOOK once in a while, you might learn something!

Indeed...

I'm to bored to figure out who started this D@$$ measuring contest, but think you need look at your source a little closer.

Additionally, a reference going out of scope will never cause a destructor invocation. The temporary object going out of scope in "const ThreadLock& a = ThreadLock()" is what causes the destructor invocation.

Pop quiz, what's the output from this app:

#include <iostream>



struct Foo

{

  ~Foo()

  { std::cout << "~Foo" << std::endl; }

};



int main(int, const char*[])

{

  Foo const& ref1 = Foo();

  Foo const& ref2 = ref1;

  Foo const& ref3 = ref2;



  return 0;

}

Answer:

~Foo

An R-value is not the same thing as a non-L-value. All L-values are also R-values. So it really should be, "it's illegal to assign a non-l-value to a reference to non-const type." A temporary is a non-l-value. The fact that it is an r-value as well isn't the issue.

it's actually a coding practice to do "Initialization is acquisition" for resources like mutexes... and when the code gets destroyed, the lock gets released...

CAPTCHA burned

PseudoNoise:

My favorite:
In Soviet Russia, the government controls commerce.

Oh you smug Americans with your oh-so-superior social structures.

You're going to have to find a more up to date straw target

Kid:

At least your drivers liscense station had such a system. My local one consisted of:

"OK. Whose next? What's your number?"
"X"
"Wait, does anybody have a number lower than X?"

There were only two of us there. (I got lucky.)

I would drop my ticket onto the floor and say, "Yes, mine is lower!" ;D

ThreadLock &a; is a syntax error in C++. You have to initialize references.

Kid:

At least your drivers liscense station had such a system. My local one consisted of:

"OK. Whose next? What's your number?"
"X"
"Wait, does anybody have a number lower than X?"

There were only two of us there. (I got lucky.)

Only two people? Are you sure you were in the right building?

Many times that I had to deal with the bureaucrats in that way, I had lines from here to Tokyo and back. (ok, not that long, but still very long)
And of course, at exactly 1700h the clerks close up, no matter how many people still need to be helped. And you need to ask for another day off at your work.

no name:

and you also have to provide a (global or class static)

#include <cstddef>



void *operator new(std::size_t, void *ptr)

{

	return ptr;

}

#include <new>