Portable C++ boost::interprocess::mutex

I was looking for long time around to have a portable robust solution for multiprocessing synchronization. Who touche this things know that good solution are boost::iterprocess named sync objects.

When your process have a named_mutex locked and your process die (there are many normal situations when process die, not just bug or others.) In that case a named_mutex will remain in locked state. There were attempt to make a robust_mutex in boost code done by Ion Gaztanaga: Robust Emulation

He had a nice idea on how to resolve abandoning state check. Each process, in game, has its own lock file and while is alive it hold that file locked. Then Ion's robust_mutex check, in case of failed lock attempt, current owner process lock file, and can determine if current mutex owner is alive or not. In case it is death mutex can be taken. The trick with file lock is nice idea cause file locks are unlocked by OS in case process die, and this seems to be well portable. This solution wraps base spin_mutex and hold current owner process id in the internal field. I made intensive testing and found 2 big problems.

-
File lock handling and way how is implemented slows down mutex in manner that it is faster just use file lock.

-
Decoupling effective lock gate variable and owner process id cause situations where mutex can be stolen by different processes.

I'm proposing solution for both problems, and I wood like to have some pro opinion about it.

-
Do not use for each existing process separate lock file but to use one file for all eventual process id (there should be enough 4MB) and for each process lock just one byte. Position of that byte is determined by the process id itself. (this is not my idea but I found it in code of Howard Chu and his excellent LMDB)

-
Do not wrap spin_mutex as is, but rewrite it's code so it use as lock gate current owner process id instead just 0/1, so lock and unlock can happen in one atomic CAS operation.

I did a try to implement it

I can't say for the thing as a whole, but there are a few things that you may improve:

-
First, this macro:

#if defined(_DEBUG) && defined(_ROBUST_MUTEX_TRACE) 
#define IPC_DBG_TRACE(msg) { std::stringstream __str; __str << bipc::ipcdetail::get_current_system_highres_count() << " " << msg; xw::ipc::dbg_tracer::trace(__str); }
#else
#define IPC_DBG_TRACE(msg)
#endif

It could use the do { / ... / } while (0) construct to avoid some rare but nasty problems with semi-colons and compiler warnings. The answers to this question provide more information. The macro XW_IPC_HANDLE_BROKEN_STATE could also use some of those, just in case someone thinks it is a good idea to use the name oldState somewhere else.

-
This expression seems a little bit overkill:

std::string mainFileName = (lockNameBase + "/main.lck").c_str();

Couldn't use simply write the following line instead? I suppose you have more chances to benefit from copy elision if you don't make several type transformations:

std::string mainFileName = lockNameBase + "/main.lck";

I know that the temporary created with operator+ should exist until the end of the expression. I don't know however whether the initialization syntax using = is part of the expression; I don't think that your code has undefined behaviour.

-
Use the standard library algorithms when you can, especially if they lower the cognitive burden. For example, instead of this conditional:

_name[len < MAX_PATH ? len : MAX_PATH] = 0;

You can use std::min:

_name[std::min(len, MAX_PATH)] = 0;

While it does not make a big difference, it really helps when you read it. I had to mentally parse the expression to know whether it did a min or a max while std::min makes it impossible to misread what's being done.

StackExchange Code Review Q#57083, answer score: 2