Semaphore use in NiftyPerforce is incorrect

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Queue up a bunch of p4 commands before the thread has a change to work
on any of them.  A good way of doing this is opening a bunch of files,
making changes to all of them, and then hitting the "checkout edited files"
button.

What is the expected output? What do you see instead?

All of the commands should eventually get run.  Instead, the first command
is run, and every time you add more commands to the queue a single command
from the front of the queue is run.

The problem is this line in P4Operations.cs:
static private Semaphore m_startEvent = new Semaphore(0, 1);

It only allows the semaphore value to increment to 1, so multiple increment
operations are clamped to 1.  Changing this to:
static private Semaphore m_startEvent = new Semaphore(0, 99999);

works like a charm.

Original issue reported on code.google.com by josh.faust@gmail.com on 11 Jun 2008 at 10:45

[GoogleCodeExporter]

Hm. It was a while I wrote that code so it actually looks a little bit hooky to 
me
now (and I've not seen C# code in quite a while). 

But, the intent of the semaphore was to make sure that the helper thread that 
does
all the commands doesn't go in an infinite loop, but rather sits and sleeps 
until the
main thread nudges it. It seems that that should work ok to allow the semaphore 
to
have higher values since it seems to be the max depth of the queue the max 
semaphore
value describes. I'll see if I can get the fix into the main line.

Original comment by jim.tila...@gmail.com on 12 Jun 2008 at 6:08

[GoogleCodeExporter]

I've been running with that change for a few months now and things work great 
-- I
tried to submit the bug when I first found it, but submitting new issues was 
offline
for a few days and I forgot about it.

The alternative would be to do something like the following (pseudo code 
because I'm
at home and don't have the code handy):

while (threadShouldContinue)
{
  m_semaphore.WaitOne();

  while (!m_queue.Empty())
  {
    ... Process p4 commands ...
  }
}

Thanks for the quick response!

Original comment by josh.faust@gmail.com on 12 Jun 2008 at 6:26

[GoogleCodeExporter]

The code is now checked in and in the 1.0.3 release. Thanks for the catch!

Original comment by jim.tila...@gmail.com on 29 Jun 2008 at 6:41

[GoogleCodeExporter]

Original comment by jim.tila...@gmail.com on 29 Jun 2008 at 6:42

• Changed state: Fixed