Use a named mutex to avoid race conditions

This is a better solution for terminating the engine if one is running already.

This is the previous review:
http://codereview.adblockplus.org/10809053/

[Wladimir Palant, 2013-06-07 14:32:01]

LGTM with the comment below addressed.

http://codereview.adblockplus.org/10891004/diff/1/src/engine/main.cpp
File src/engine/main.cpp (right):

http://codereview.adblockplus.org/10891004/diff/1/src/engine/main.cpp#newcode201
src/engine/main.cpp:201: }
Remove the block of code above?

[Felix Dahlke, 2013-06-07 14:33:24]

http://codereview.adblockplus.org/10891004/diff/1/src/engine/main.cpp
File src/engine/main.cpp (right):

http://codereview.adblockplus.org/10891004/diff/1/src/engine/main.cpp#newcode201
src/engine/main.cpp:201: }
On 2013/06/07 14:32:01, Wladimir Palant wrote:
> Remove the block of code above?

What in particular? The check for ERROR_ALREADY_EXISTS? That's necessary.
CreateMutex will open an already existing mutex, but set last error to that.
That's how we can figure out if there was one already.

[Wladimir Palant, 2013-06-07 14:46:45]

http://codereview.adblockplus.org/10891004/diff/1/src/engine/main.cpp
File src/engine/main.cpp (right):

http://codereview.adblockplus.org/10891004/diff/1/src/engine/main.cpp#newcode201
src/engine/main.cpp:201: }
Ok, that debug message is misleading then. How about:

if (!mutex || GetLastError() == ERROR_ALREADY_EXISTS)
{
  DebugLastError("Failed to create a mutex, another engine instance appears to
be running");
  return 1;
}

[Felix Dahlke, 2013-06-07 14:48:38]

On 2013/06/07 14:46:45, Wladimir Palant wrote:
> http://codereview.adblockplus.org/10891004/diff/1/src/engine/main.cpp
> File src/engine/main.cpp (right):
> 
>
http://codereview.adblockplus.org/10891004/diff/1/src/engine/main.cpp#newcode201
> src/engine/main.cpp:201: }
> Ok, that debug message is misleading then. How about:
> 
> if (!mutex || GetLastError() == ERROR_ALREADY_EXISTS)
> {
>   DebugLastError("Failed to create a mutex, another engine instance appears to
> be running");
>   return 1;
> }

I think it makes sense to have two different messages, it's two entirely
different things. The first says the mutex could not be created/opened (rare,
crazy failure), the second one is a pretty normal thing to happen.

[Oleksandr, 2013-06-07 14:52:09]

http://codereview.adblockplus.org/10891004/diff/1/src/engine/main.cpp
File src/engine/main.cpp (right):

http://codereview.adblockplus.org/10891004/diff/1/src/engine/main.cpp#newcode190
src/engine/main.cpp:190: AutoHandle mutex(CreateMutexW(0, false,
L"AdblockPlusEngine"));
Shouldn't this be Global\AdblockPlusEngine?

[Felix Dahlke, 2013-06-07 15:13:54]

On 2013/06/07 14:52:09, Oleksandr wrote:
> http://codereview.adblockplus.org/10891004/diff/1/src/engine/main.cpp
> File src/engine/main.cpp (right):
> 
>
http://codereview.adblockplus.org/10891004/diff/1/src/engine/main.cpp#newcode190
> src/engine/main.cpp:190: AutoHandle mutex(CreateMutexW(0, false,
> L"AdblockPlusEngine"));
> Shouldn't this be Global\AdblockPlusEngine?

Should it? The examples on MSDN don't do this. Is that something semantic or
just convention?

[Oleksandr, 2013-06-07 15:19:49]

On 2013/06/07 15:14:02, Felix H. Dahlke wrote:

Sorry, I think Global isn't actually needed here. It would create mutex for all
users, which is not what we want in this case. So LGTM.

[Felix Dahlke, 2013-06-07 15:40:30]

Pushed this, so it can go into the next dev build.

Leaving the issue open though, Wladimir didn't LGTM the latest version.

[Felix Dahlke, 2013-06-11 07:04:05]

Wladimir said he's fine with the two messages, closing this.

[Wladimir Palant, 2013-06-11 07:04:54]

On 2013/06/07 15:40:30, Felix H. Dahlke wrote:
> Leaving the issue open though, Wladimir didn't LGTM the latest version.

I didn't mean to revoke LGTM from the previous comment, this was merely a nit.