|
You can use Sleep( 0 ) instead of SwitchToThread().
|
|
|
|
|
Switch(0) will switch another process SwitchToThread() only causes another thread to scheduled ... in theorie.
bkausbk
|
|
|
|
|
The reason I am asking this is that I found the following 3 problems.
1. First, the constructor. The parameter is just a pointer to a lock variable. You did not say that the variable has to be initialized to 0. But if it is not initialized to 0, then every thread calling the lock method will go into an infinite loop. Ok, maybe you just missed this.
2. Secondly, the lock method cannot be called by the same thread again, otherwise it goes into an infinite loop. This may be a defect instead of a bug. The problem is you did not say it in the article (that the lock method cannot be called twice by the same thread).
3. Finally, if a thread called the lock method successfully, a different thread can unlock the same resource (because your has_lock method is buggy, as pointed out in another comment).
Sorry for sounding too hush, I just had a bad day.
|
|
|
|
|
Oops, you did say the following so forget about the first problem I mentioned:
The availability of the resource is defined by the value of shared variable. If the value of this variable is zero, then the resource is available, otherwise it is in use by another thread.
|
|
|
|
|
1. As you say, this is in the documentation
2. I would not consider this a bug with the implementation. Perhaps I should have documented it better, but hey, I can't think of every eventuality A spin lock should only ever be held locked for a very short time, and by implication would not be nested. If the lock is held for any significant amount of time, then other threads would consume high CPU cycles while waiting for the lock, and this would be incredibly inefficient. In such a case, an Event would be more appropriate. Sorry for not being specific about this.
3. Yes, the has_lock() method actually implements an is_locked(), whereby it returns true if the lock is held by _any_ thread. Ooops Getting the class to perform this level of validation is, I think, too much. The spin lock implementation is designed to be rapid, and we are getting way too detailed with this level of functionality. At some point, the programmer must take responsibility for writing solid code, and I think this is true in this case. Again, perhaps the documentation let me down here. Who _likes_ writing documentation?
|
|
|
|
|
This function doesn't work as it was apparently intended and documented. As written, it only tests whether the lock is held by anyone, not by the particular object doing the check. This is a pretty obvious bug.
--
Eric
Move along, nothing to see here.
|
|
|
|
|
Yes, the has_lock() method actually implements an is_locked(), whereby it returns true if the lock is held by _any_ thread. Ooops, sorry for any confusion.
|
|
|
|
|
You wrote :-
“the shared variable hold the value 0x6b636f6c. This will read "lock" if it is cast to a char *”
This is not correct. For it to read “lock” when casted to char *, it should be stored in 4 consecutive locations
Thought I’d mention it here
Regards,
Nish
Native CPian.
Born and brought up on CP.
With the CP blood in him.
|
|
|
|
|
Hi Nish,
Sorry, I wasn't clear in there. The value 0x6b636f6c represents the characters 'k', 'c', 'o' and 'l' respectively. If you take the address of the variable and cast it to a char*, you will see "lock..." in the debugger, where ... is extra characters not interested in.
try this to see what I mean, and inspect the contents of p
DWORD dw = 0x6b636f6c;
char *p = (char *)&dw;
This is not overly useful, but I had to define a non-zero value for the lock value, so thought this as good as any.
Thanks for you comment.
-- Craig
|
|
|
|
|
Craig, don't think bad about the critics , actually when somewhere in time , I'd post a threading article, I'll expect to be critized from all the sides
Multithreading is a fascinating subject, that I love to read about and where I had a lot of practice on a previous job, and is a field that we are learning almost all of the days.
I actually learned a lot discussing and watching others discussing multithreading subjects here on Code Project .
Your class , definitely has his use , man
Cheers,
Joao Vaz
A person who is nice to you, but rude to the waiter, is not a nice person - Natalie Portman (Padme/Amidala of Star Wars)
|
|
|
|
|
Craig Henderson wrote:
char *p = (char *)&dw;
Oh okay
That is correct
I thought you meant this :-
char *p = (char *)dw; //it compiles I think but wrong
Sorry for misinterpreting you Craig
Nish
Regards,
Nish
Native CPian.
Born and brought up on CP.
With the CP blood in him.
|
|
|
|
|
Hi,
spin locks are popular.
this page shows an elegant multiple-readers single-writer lock similar to the one shown in the article above.
Cheers
--
Maxime Labelle
maxime.labelle@freesurf.fr
|
|
|
|
|
You should use FreeLibrary within your destructor , this is necessary in at least on Win95 and Win98.
Besides ,you could be used the Sleep(0) instruction to abdicate thread time without SwitchToThread ... You have another drawback ,that is as long as the spinlock is owned other waiting threads are spinning and using valuable CPU time doing nothing useful.
A more complex , but superior aproach would be creating a waiters variable , a event object , a own avariable and a n spin retries , this number is tricky, because it should be less than it takes to do a full thread or process context switch and is should be long enough to prevent the situation when the spinlock is going to be released within a few microseconds.
Steps:
Requesting the spinlock.
First the lock function should start spinning n spin counts, this number is tricky to choose , because the time spent spinning should be less than it takes to do a full thread/process/context-switch and is should be long enough to prevent the wait if the spinlock is going to be released within a few microseconds.
something like this :
for(unsigned spin=0; spin<spin_retries; spin++) {
if(InterlockedExchange(m_plock,1)==0)
return;
}
then
the waiters variable is incremented (must be done with InterlockedExchange() since other threads may be modifying it at the same time). Then it the spinlock tries to obtain the spinlock with a InterlockedExchange(). If that succeeds waiters is decremented and the function returns. If it does not succeed it blocks on the event , then resets the event and goes back to trying to obtain the spinlock.
something like this:
InterlockedIncrement(&waiters);
for(; ; ) {
if(InterlockedExchange(m_pLock,1)==0) {
InterlockedDecrement(&waiters);
return;
}
WaitSingleObject(hEvent, ...);
ResetEvent(hEvent);
}
the complete picture of the body of lock function could be something like this
for(unsigned spin=0 ; spin<spin_retries; spin++) {
if(InterlockedExchange(m_plock,1)==0)
return;
}
InterlockedIncrement(&waiters);
for(; ; ) {
if(InterlockedExchange(m_pLock,1)==0) {
InterlockedDecrement(&waiters);
return;
}
WaitSingleObject(hEvent, ...);
ResetEvent(hEvent);
}
Releasing the spinlock.
First ownership is released by setting owned to 0. Then if there are any waiters (they may be blocked on the manual reset event ) the event is posted to wake them up.
the unlock function could be something like this:
owned = 0;
if(waiters!=0)
SetEvent(hEvent);
This kind of implementation has the following advantages:
-> requesting an unowned spinlock takes only a few instructions
-> requesting an owned spinlock may not need an expensive API call
-> releasing a spinlock that no one is waiting for is fast
-> valuable CPU time is not spent spinning forever
Disadvantages : at least one, this provoques a deadlock, concerning nested ownership, aka , requesting a spinlock that you already have causes ... deadlock ... this sincerily is to hard for me to resolve since, I don't actually known that much about Multithreading , and this kind of stuff is hard as is , so I currently don't know how to solve it efficiently
If someone knows I much like to know ...
Cheers,
Joao Vaz
A person who is nice to you, but rude to the waiter, is not a nice person - Natalie Portman (Padme/Amidala of Star Wars)
|
|
|
|
|
Isn't this kind of what CRITICAL_SECTION does for you? If I remember correctly, CRITICAL_SECTION uses a spin lock techinique first, and will only switch into using a kernel mode object if it's already owned(kinda like what you've described). You don't have to worry about nested ownership w/ CRITICAL_SECTION either.
FYI, CRITICAL_SECTIONs execute in user mode & don't require the expensive switch to kernal mode that using events, mutexes, or semephores require. I think the switch to kernel mode requires 1000 CPU instructions(or something like that). Using WaitForSingleObject/WaitForMultipleObjects() causes the switch to kernel mode.
Still, a simple spin lock class has its uses.
-Wes
Sonork ID 100.14017 wtheronjones
|
|
|
|
|
CRITICAL_SECTIONs only spin on multi-processor PCs AND they do go to kernel mode when there is contention for the lock.
|
|
|
|
|
Besides of Matt said, This is a exercise of a Spin Lock , not a critical section ...
Wes Jones wrote:
simple spin lock class has its uses.
Of course , it have , I never said the opposite ! Every simple pice of multithreaded code have his place , I talked about a more general Spin lock implementation ! I listed the advantages that the approach had.
Wes Jones wrote:
I think the switch to kernel mode requires 1000 CPU instructions
You are totally correct on this
Cheers,
Joao Vaz
A person who is nice to you, but rude to the waiter, is not a nice person - Natalie Portman (Padme/Amidala of Star Wars)
|
|
|
|
|
|
Very interesting article. I have not seen this before. It looks very promising. Have you used the meterd sections? If so, how do you like them?
|
|
|
|
|
Matt Gullett wrote:
Very interesting article. I have not seen this before. It looks very promising. Have you used the meterd sections? If so, how do you like them?
I hadn't seen this before either and it is indeed very interesting. I will check it out next time I need something like this. Being able to use a count is what interests me most right now. I'm also wondering whether this would be a good base for a multiple reader/single writer lock object!
ED4W makes extensive use of multithreading, Critical Sections, Events etc. Anything else I can throw into the bag which will make my multithreaded life a bit simpler is most welcome.
Neville Franks, Author of ED for Windows. www.getsoft.com
|
|
|
|
|
Neville Franks wrote:
Anything else I can throw into the bag which will make my multithreaded life a bit simpler is most welcome.
Agreed.
|
|
|
|
|
Neville Franks wrote:
I'm also wondering whether this would be a good base for a multiple reader/single writer lock object!
Hi, Neville, you should try Valery Prymakov lock at Valey MSRW lock
Check his _mrsw_guard.h
His multiple reader/single writer lock object is faster than Jeffrey Richter one that shows up on Programming windows, 4ed .
And as Maxime Labelle, pointed out another one is at Brad wilson C++ light lock
Both of this mrsw locks were discussed at ATL mailing lists, so you should have a good start. Attention, the Brad's one dealock if you have a reader lock and you ask a writer lock, this is pointed out by him.
Also do a search on ATL Mailing lists for MRSW
Have fun
Cheers,
Joao Vaz
A person who is nice to you, but rude to the waiter, is not a nice person - Natalie Portman (Padme/Amidala of Star Wars)
|
|
|
|
|
You should use FreeLibrary within your destructor , this is necessary in at least on Win95 and Win98.
Where have you seen this stated? I know it was the case for Win3.x, but I have never been able to locate a reference that it has every been required for Win32. Microsoft people have even verified this for me.
Tim Smith
I know what you're thinking punk, you're thinking did he spell check this document? Well, to tell you the truth I kinda forgot myself in all this excitement. But being this here's CodeProject, the most powerful forums in the world and would blow your head clean off, you've got to ask yourself one question, Do I feel lucky? Well do ya punk?
|
|
|
|
|
Tim Smith wrote:
Win95 and Win98.
If I recall correctly, from Microsoft !!! Most of the times, isn't necessary, since the Windows automatically takes care of this, but I had 2 problems with not calling Freelibrary ... but I quite not remember the context,but I remember that was important in this 2 cases . Damn memory
Cheers,
Joao Vaz
A person who is nice to you, but rude to the waiter, is not a nice person - Natalie Portman (Padme/Amidala of Star Wars)
|
|
|
|
|
I think that it had to do with race conditions on a multithreaded server , where I controled the runtime linking with loadlibrary and the other with raw com , since LoadLibrary increments the ref count and FreeLibrary decrements it ...
Cheers,
Joao Vaz
A person who is nice to you, but rude to the waiter, is not a nice person - Natalie Portman (Padme/Amidala of Star Wars)
|
|
|
|
|
Interesting. I was maining thinking of the case of the DLL still being in memory after process exit. But from you other message I gather that wasn't what you were talking about.
Tim Smith
I know what you're thinking punk, you're thinking did he spell check this document? Well, to tell you the truth I kinda forgot myself in all this excitement. But being this here's CodeProject, the most powerful forums in the world and would blow your head clean off, you've got to ask yourself one question, Do I feel lucky? Well do ya punk?
|
|
|
|
|