 |
|
 |
Here is a scenario that does not appear to work:
I'm want to wait for an event to occur twice. I attempted this by creating a semaphore with a total count of two and an initial count of zero. My event comes in and releases the semaphore. I attempt to wait for the event by using AcquireAll(). This should theoretically cause my thread to pend until I obtain a count of two (essentially my event occurring twice). This does not happen. AcquireAll returns after obtaining a single slot instead of all.
1) Thread A: Create semaphore with a max value of 2 and an initial value of 0.
2) Thread B: Event occurs releasing semaphore (release(1))
3) Thread A: Call AcquireAll()
4) Thread A: AcquireAll() comes back with apparent success, however, I never obtained both slots!
This appears to be caused by the while loop that checks while (count == 0) inside the Acquire(int timeout) method. Since the count is already 1 because the event has occurred once, this method returns true which makes AcquireAll return with apparent success.
To test this scenario, I created Thread A and Thread B as specified earlier. I ensured that ThreadB's event only happens one, thus my ThreadA should pend forever. It does not.
|
|
|
|
 |
|
 |
cbye, see my "What Wait Does" post just below this one. I think that may be your problem also. Does AquireAll() return false (i'm guessing so)?
|
|
|
|
 |
|
 |
Actually AquireAll returns true! I believe the reason for this is because in my example Release(1) occurs BEFORE AquireAll is even called. This appears to be a race condition in the implementation logic. If the Release occurs before the AquireAll the embedded Acquire method will return true without even attempting to take the semaphore.
|
|
|
|
 |
|
 |
Perhaps your problem lies in the timeout logic, it appears that the code in AquireAll(millisecondTimeout) does not check for Timeout.Infinite (passed in from AquireAll()) before it starts subtracting the time elapsed. That make timeout < 0 and so it's sets it to 0.
The first "count" is aquired (true) then it checks for the remaining time to wait, hmmmmmm....
timeout = millisecondsTimeout - elapsedMS;
where millisecondsTimeout = Timeout.Infinite = -1
which makes the next timeout value = 0, which should cause Aquire(int millisecondsTimeout) to return false (according to my tests, since the lock was not reaquired before the timeout period).
if a check for millisecondsTimeout being >= 0 before doing any calculations on the remaining timeout left, it should wait forever on the second (2nd) Release.
|
|
|
|
 |
|
 |
in the code:
if (!Monitor.Wait(syncLock, millisecondsTimeout))
return false;
Why do you return false? The lock was granted or it would not return.
The millisecond timeout is only how long to remain in the wait queue before being put into the ready queue (a timeout on getting signaled, so if a signal was missed you can still regain the lock). The method Wait will not return until the lock is aquired (Semaphore.Aquire(0) returns false because you are put on the ready queue immediately, I think)
.NET documentation says:
Wait
Return Value
Type: System..::.Boolean
true if the lock was reacquired before the specified time elapsed; false if the lock was reacquired after the specified time elapsed. The method does not return until the lock is reacquired.
Am I wrong in my interpretation of this?
|
|
|
|
 |
|
 |
Okay, it appears that the problem is in the way the timeout is decremented in AquireAll(timeout), when timeout.Inifinite is used as in the AquireAll() (without parameter) is done. Should not subtract elapsed time from timeout.Inifinite (-1).
|
|
|
|
 |
|
 |
I'd like to use this in a commercial project and wanted to know if they're any licensing restrictions? If so, I'm going to have to write my own version which I really don't want to do. In return, I'll keep your name quoted as the author of the article. Please reference this license[^] for an example.
Thanks...
|
|
|
|
 |
|
 |
First, a compliment: This is very impressive. Thanks!
Now the question:
This code:
elapsedMS = (int)((TimeSpan)
(DateTime.Now - start)).TotalMilliseconds;
timeout = millisecondsTimeout - elapsedMS;
doesn't look right to me.
Shouldn't it be:
elapsedMS = (int)((TimeSpan)
(DateTime.Now - start)).TotalMilliseconds;
timeout = timeout - elapsedMS;
so that each time through the loop it will get smaller and smaller?
also, strictly speaking, it wasn't necessary to create the timeout variable since millisecondsTimeout is byValue, but that's a nit-pick, to be sure.
|
|
|
|
 |
|
 |
How to release if anyone is waiting into Acquire? Both need to hold the lock of syncLock!!
|
|
|
|
 |
|
 |
Could you expand on that? Did not follow that. Thanks.
--
William Stacey, MVP
http://mvp.support.microsoft.com
|
|
|
|
 |
|
 |
public bool Acquire(int millisecondsTimeout)
{
lock(syncLock)
{
...snip...
Monitor.Wait(syncLock, millisecondsTimeout)
...snip...
}
}
public void Release(int releaseCount)
{
...snip...
lock(syncLock)
{
...snip...
Monitor.PulseAll(syncLock);
...snip...
}
}
To aquire a lock on the semaphore, you need to obtain syncLock (which is released the moment you leave the function). If there are no slots available, you block at the Monitor.Wait() call, INSIDE the lock statement. When a different thread finishes up and tries to release, it attempts to aquire the exclusive lock on syncLock. Since there is a thread blocking on Monitor.Wait() inside the lock, this won't release.
I've attempted to create this deadlock, but seem unable. I believe this is what the original poster was saying.
Charles
|
|
|
|
 |
|
 |
Hi Charles. That is not accurate.
The implementation is correct and a well known pattern in this type of producer-consumer locking. Monitor.Wait releases the lock so a thread calling Release is not blocked. The lock will be reacquired for the Acquire thread *before Monitor.Wait returns (unless there is a timeout or some mutex exception) - so the rest of the code is still "inside" the lock. The monitor queue itself will determine which thread gets the lock after a Pulse operation - but only one thread will receive the lock and be able to continue on. So the pattern is sound and well-known (that is why you could not create a deadlock).
--
William Stacey, MVP
http://mvp.support.microsoft.com
|
|
|
|
 |
|
 |
In the Acquire(ms) function it is possible to wait much longer than necessary for the ms timeout. If a waiting thread gets signaled, but is barged by another thread, the waiting thread will re-enter the while loop and wait the full time again before failing, and this can happen indefinatly.
|
|
|
|
 |
|
 |
Nice work. But isn't 1 a valid maxCount? ... if ( maxCount < 1 ) throw new ArgumentOutOfRangeException("maxCount must be > 1."); DrGary83
|
|
|
|
 |
|
 |
A maxCount of 1 would make it a binary semaphore and not a counting semaphore.
Though I will say it's worth doing just that since my own version of semaphore implementation operate as counting or binary in a similar fashion.
Regards.
ViperX
|
|
|
|
 |
|
 |
1 is a valid maxCount (just not less then 1). The message string is wrong as you point out. The message instead should read "maxCount must be >= 1." I guess. I will make the change and other changes y'all see. Thanks.
--
William Stacey, MVP
|
|
|
|
 |