Root Lockguard

[ferdymercury]

• Checked for duplicates

Describe the bug

I've been using in my programs these constructs:

TMutex myMutex;
//...
R__LOCKGUARD(myMutex)

(In analogy with https://en.cppreference.com/w/cpp/thread/lock_guard)

Somehow, in one of my computers (not in the other, not sure if it is related with an update I did in ROOT or something similar), I get a compilation error. This error didn't appear before.

error: no matching function for call to ‘TLockGuard::TLockGuard(TMutex&)’
     R__LOCKGUARD(fGuiMutex) // scoped mutex
     ^
/opt/root_src/core/base/inc/TVirtualMutex.h:76:4: note: candidate: TLockGuard::TLockGuard(TVirtualMutex*)
    TLockGuard(TVirtualMutex *mutex)
    ^~~~~~~~~~
/opt/root_src/core/base/inc/TVirtualMutex.h:76:4: note:   no known conversion for argument 1 from ‘TMutex’ to ‘TVirtualMutex*’
error: expected ‘,’ or ‘;’ before

72:4: note: candidate constructor not viable: no known conversion from 'TMutex' to 'const TLockGuard' for 1st argument

Expected behavior

No error is produced as before.

Or why are the others compiling well ?

Should I change instead to R__LOCKGUARD(&myMutex); ?
If yes, could this be added into the documentation ? Right now, it is not specified if it has to be a pointer or not.

To reproduce

• Open a ROOT terminal
• TMutex fr
• R__LOCKGUARD(fr)

Setup

   ------------------------------------------------------------------
  | Welcome to ROOT 6.25/01                        https://root.cern |
  | (c) 1995-2021, The ROOT Team; conception: R. Brun, F. Rademakers |
  | Built for linuxx8664gcc on Jul 08 2021, 11:17:13                 |
  | From heads/master@v6-25-01-1584-g068c20d159                      |
  | With c++ (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0                     |
  | Try '.help', '.demo', '.license', '.credits', '.quit'/'.q'       |
   ------------------------------------------------------------------

[ferdymercury]

I noticed that in one computer, _REENTRANT is defined, but not in the other, not sure why, even if they are both Ubuntu.
https://github.com/root-project/root/blob/master/core/base/inc/TVirtualMutex.h#L90

So my original code only worked if _REENTRANT was undefined, so maybe it wasn't doing anything at all? I don't remember however having seen any warning ::Fatal("Init","_REENTRANT must be #define-d for TThread to work properly."); The threads and mutexes worked quite well despite of some smaller issues (#8365)
Maybe related: https://root-forum.cern.ch/t/trentrantrwlock-thread-lock-program-freezes/45116/14

Some other questions:

• Why is the version with the non-pointer not supported as in std::lockguard?
• Why does one version need ";" at the end, but not the other one?
• How can I force REENTRANT to be defined when including ROOT classes from external projects via CMake find_package?
• Why do two Ubuntu computers behave differently, and why did the behaviour change (the error appear) recently only in of both?

[couet]

I am not sure who can look at this. I assigned it to @Axel-Naumann for the time being.

[Axel-Naumann]

Why is the version with the non-pointer not supported as in std::lockguard?

Because ROOT traditionally passes pointers. You are welcome to propose a PR, I think this would be a fine and minimal addition. It might be more useful to make sure that the argument is a pointer to a TVirtualMutex even in the non-_REENTRANT case, though - for which I'd also welcome a PR!

Why does one version need ";" at the end, but not the other one?

Please just provide ; after R__LOCKGUARD(mutex). Alternatively, please submit a PR that adds ; {} behind the current "needs a semicolon". But as this isn't really a user facing interface I don't think it really matters.

How can I force REENTRANT to be defined when including ROOT classes from external projects via CMake find_package?

You want to pass -pthread to compiler and linker - without that, no threading support can exist in your code, and thus locking cannot work.

Why do two Ubuntu computers behave differently, and why did the behaviour change (the error appear) recently only in of both?

That's an excellent question to which I do not have an authoritative answer. My assumption is that _REENTRANT was off and thus the conversion from TMutex& to TVirtualMutex* wasn't needed.

Let me know if there is something left for us to do to fix this issue!

[pcanal]

Why is the version with the non-pointer not supported as in std::lockguard?

In addition to what @Axel-Naumann mentioned, ROOT supports a run-time switch where no lock is taken ('thread safety is not enabled') and this is indicated by the pointer being nullptr (thus avoid even the construction of the mutex when they are not needed).