Hi all!

This is a simple piece of code that I wrote about a year ago, in response to a question on Daniweb. I decided to infuse it with some C++11 magic, which led to a pretty sweet code snippet, full of neat little techniques you might want to use, not to mention that this class can actually be pretty handy to have. There are a few similar implementations out-there, but they all use a different approach, my main inspiration is the scope-guard technique.

This is an implementation of a simple wrapper class template that allows a variable to be externally made thread-safe via a mutex lock. This can be useful when a single variable (e.g. non-trivial object, like a std::string or std::vector) is of a class that has no built-in thread-safety, when thread-safety is required. The typical solution is to pair the object with an associated mutex and lock it during any thread-sensitive operations. For example, if the object is a std::vector, one could wrap all calls that rely on the underlying elements (like accessing them or insertions) with a mutex lock. Although this is typically the best solution in terms of fine-grained control over when threads can get blocked, it can be cumbersome to implement in general (at worst, creating a "completely thread-safe std::vector" would involve wrapping every member function in a mutex lock).

This simple wrapper superposes a scoped locking mechanism on top of an underlying object. This forces all the operations on the object to be mutex-locked, within a scope, such that any operations done within that scope are thread-safe. Obviously, this provides a very coarse-grained thread-safety and may not be the best solution, performance-wise, in most applications. However, it is quick and simple, and reduces clutter in classes that would otherwise have many pairs like var1 and var1_mutex, var2 and var2_mutex, etc.

This wrapper template works as follows. First, it privately wraps the actual object along with a mutex for it. Then, the object can only be indirectly accessed via the creation of a const or non-const locked-pointer to the object. The locked-pointer is non-copyable, non-assignable, immovable, and always const. This prohibits most unintentional misuses on the user-side (but not dirty hacks, of course!). The locked-pointer can be used as an ordinary pointer to refer to the object. Because references to the variable can be given by the lock, there are ways in which a reference could be carried out of the scope of the lock, and thus, enabling access to it in a thread-unsafe manner. However, such a trick would appear obvious (violating common practice in C++) and could hardly go unoticed or be unintentional. Finally, the wrapper relies on the binding of a temporary to a const reference to restrict the scope of the lock without allowing copy-constructor or assignment to anything outside the scope of original lock (this is basic scope-guard technique).

N.B. for Compilation: The code requires pretty good C++11 support. It compiles and runs without errors on GCC 4.7.0 (required for the template aliases only), it also compiles with GCC 4.6.2 when replacing the template alias with its C++03 equivalent. Of course, it requires the compilation flag -std=c++0x or -std=c++11. As for MSVC++, don't even try.

Edited 4 Years Ago by mike_2000_17: tab-space formatting

Comments
good work :)
// lockable.hpp

/*
 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF
 * ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED
 * TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A
 * PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
 * SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR
 * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
 * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE
 * OR OTHER DEALINGS IN THE SOFTWARE.
 *
 * Author: Sven Mikael Persson
 * Date: June 27th, 2012
 */

#ifndef LOCKABLE_OBJECT_HPP
#define LOCKABLE_OBJECT_HPP


#include <mutex>
#include <type_traits>


template<typename T>
class lockable;  // forward-declaration.

namespace detail {
  
  /*
   * This class template is a locked smart-pointer used under-the-hood.
   */
  template <typename T>
  class locked_ptr_impl {
    private:
      mutable std::unique_lock<std::mutex> l;
      T* ptr;
      
      // private constructor:
      locked_ptr_impl(std::unique_lock<std::mutex>&& aLock, T* aPtr) : 
                      l(std::move(aLock)), 
                      ptr(aPtr) { };
      
      // private move-constructor:
      locked_ptr_impl(locked_ptr_impl&& rhs) : 
                      l(std::move(rhs.l)), 
                      ptr(rhs.ptr) { };
	  
      // private constructor:
      explicit locked_ptr_impl(std::mutex& aMut, T* aPtr) : 
                               l(aMut), ptr(aPtr) { }; 
    public:
      locked_ptr_impl(const locked_ptr_impl& rhs) = delete;         //non-copyable
      locked_ptr_impl& operator=(const locked_ptr_impl&) = delete;  //non-copy-assignable
      locked_ptr_impl& operator=(locked_ptr_impl&&) = delete;       //non-move-assignable
      
      // Implicit conversion to the corresponding const smart-pointer:
      operator locked_ptr_impl<const T>() {
        return locked_ptr_impl<const T>(std::move(l), ptr); // <-- allowed by friendship.
      };
      
      // Friendship with non-const smart-pointer:
      friend class locked_ptr_impl< typename std::remove_const<T>::type >;
      // Friendship with the mutex wrapper class:
      friend class lockable<T>;
      
      // Dereference operator overload:
      T& operator*() const { return *ptr; };
      // Pointer-member access operator overload: 
      T* operator->() const { return ptr; }; 
  };
  
};

// Template alias for the public interface to the under-the-hood smart-pointer:
template <typename T>
using locked_ptr = const detail::locked_ptr_impl<T>&;

// For GCC version < 4.7.0, use this replacement for the template alias:
// template <typename T>
// struct locked_ptr {
//   typedef const detail::locked_ptr_impl<T>& type;
// };


/*
 * This class template is the main wrapper to associate a mutex to an object.
 */
template<typename T>
class lockable {
  private:
    mutable std::mutex mut;
    T value;
  public:
    // Forwarding constructor for ease of construction of the wrapped value:
    template <typename... Args>
    explicit lockable(Args&&... arg) : mut(), value(std::forward(arg)...) { }; 
    
    // Copy-constructor (copies only the value, generates a new mutex):
    lockable(const lockable<T>& rhs) : mut(), value(*(&rhs)) { };
    
    // Swap function (only swaps the value, not the mutex):
    friend void swap(lockable<T>& lhs, lockable<T>& rhs) {
      std::unique_lock<std::mutex> l1(lhs.mut), l2(rhs.mut);
      using namespace std;
      swap(lhs.value, rhs.value);
    };
    
    // Copy-and-swap (and, implicitly, move-and-swap):
    lockable<T>& operator=(lockable<T> rhs) { 
      swap(*this, rhs);
      return *this;
    };
    
    // Non-const address-of operator to obtain a locked pointer:
    detail::locked_ptr_impl<T> operator&() { 
      return detail::locked_ptr_impl<T>(mut,&value);
    };
    
    // Const address-of operator to obtain a locked pointer:
    detail::locked_ptr_impl<const T> operator&() const { 
      return detail::locked_ptr_impl<const T>(mut,&value);
    }; 
};


#endif


// test.cpp  (test program)

#include "lockable.hpp"

#include <iostream>
#include <string>
#include <thread>

lockable<std::string> my_str;     //create a lockable variable of type std::string.
std::string my_result_str;        //result of the test.

void printMyStr() {
  locked_ptr<const std::string> s = &my_str;  // locks the string.
  std::cout << *s << std::endl;               // prints its value.
  my_result_str += *s;                        // appends its value to the result.
};

int main() {
  std::thread* t;
  {
    locked_ptr<std::string> s = &my_str;  // locks the string.
    t = new std::thread(printMyStr);      // thread started, will be blocked until lock is released (end of scope).
    
    my_result_str = (*s = "Hello "); // set the first half of the message.
    std::cout << *s;                 // print the first half.
    *s = "World";                    // set the second half of the message.
  };                                 // release the lock on my_str (that should allow the other thread to unblock).
  t->join();                         // wait for the other thread to be finished.
  delete t;
  if(my_result_str == "Hello World") // check the result, should be "Hello World" if the threads were in sync.
    return 0;
  else
    return 1;  // error, my_result_str should be "Hello World" if the synchro worked correctly.
};

Career objective:
To lead research and development of robotics applications for consumers, industrials or the aerospace industry; to best utilize multidisciplinary competences within a research or development team; and to help use robotics and mechatronics technologies to bring a new developmental revolution.

Expert Programming Skills: With more than 12 years experience in object-oriented programming and softwarengineering, skills range from low-level hardware programming (device drivers, buses, etc.) to high-level programming (artificial intelligence, event-driven software, etc.). Main expertise is in C/C++ with active participation to online C++ forums and have occasionally written programming tutorials and done code reviews. Very competent in many more languages including MatLab/Simulink, Delphi, LabVIEW, Java, etc.

Mechanical Engineering Skills: Competent with most CAD software (AutoCAD, Pro/E, SolidWork, etc.) as well as many simulation software (MSC - ADAMS / PATRAN / NASTRAN, SimuLink, Ansys, etc.). Skilled with mechatronics designs including design and analysis of mechanism, design optimization, electrical drives, and various actuator technologies. Expert analytical skills and numerical simulation skills for multi-body dynamics.

Control Engineering Skills: Competent and experienced with either analysis, synthesis, or implementation of control systems and software. Familiar with robust control theory, discrete-time issues, non-linear control, and probabilistic approaches to estimation and control. Strong mechatronics analytical skills for model-based control approaches and implementations.

Electrical Engineering Skills: Familiar with printed-circuit board design, circuit analysis and various low-level communication buses. Skilled with electrical drives and control circuits. Most familiar with common sensors used in robotics (lidars, computer-vision, INS, etc.).

Operating Systems: Linux, Windows, and any Unix-based system.

Other Applications: MatLab/SimuLink, LaTeX, MS Office Suite, LabVIEW, MSC ADAMS, Ansys, SolidWork, etc.

Languages: French and Swedish as mother tongues; Fluent in English, written and spoken, at mother tongue level; Functional in German, read and spoken; Basic skills in Spanish and Finnish.

Citizenship: Dual-citizenship as Canadian and Swedish.

Miscellaneous: Skilled at machining with most common machine tools, strong verbal and written communication skills, excellent troubleshooting and debugging skills, exceptional problem solving skills, and work well in a team.

If using GCC version < 4.7.0 (me 4.6.3) need to uncomment out this as Mikael Persson indicates

// For GCC version < 4.7.0, use this replacement for the template alias:
// template <typename T>
// struct locked_ptr {
//   typedef const detail::locked_ptr_impl<T>& type;
// };

And replace all

locked_ptr<const std::string>

with

locked_ptr<const std::string>::type

in the test.cpp and then it will compile.

g++ -std=c++0x test.cpp -lpthread

Edited 4 Years Ago by histrungalot: Fix

Thanks for the precision. And, for the record, with those modifications, it also compiles in GCC 4.6.2.

Having a variable named l is a bad idea.

Overloading the address-of operator is a terrible idea.

Edited 4 Years Ago by Rashakil Fol

Defining the type as detail::locked_ptr_impl for no reason is another, it could have just been locked_ptr and you wouldn't have compatibility problems.

swap(lockable<T>&, lockable<T>&) is broken.

It doesn't work properly when lhs and rhs are the same object. It tries to lock the mut field twice, but std::mutex is not recursively lockable.

Edited 4 Years Ago by Rashakil Fol

Another problem with swap is that it contains implicit behavior that lhs and rhs's mutexes are locked in a particular order. This makes the object prone to deadlock or fragile with respect to changes in code. Instead, swap should not be implemented, and a separately named function acquire_locks_left_to_right_and_swap (or something) may be implemented. That might seem bad, if you want to use some algorithm that expects a swap to exist, but, um, that's exactly what you want to avoid.

Also, I think that std::forward(arg) needs to be std::forward<Args>(arg), if you actually try to instantiate a lockable with constructor arguments. This might be only necessary in gcc 4.6.3.

Another problem with lockable::operator&, in addition to the fact that it's the most confusing possible name for such a function, is that it encourages broken use in expressions.

std::string foo = *(&my_str) + *(&my_str);

There's one example of a relatively innocuous-looking expression that will deadlock.

Instead, locked_ptr should have a public constructor that takes a lockable (or a pointer to lockable) as its argument.

If the code is forced to have locked_ptr<std::string> written out when a lock is acquired, this sort of mistake and confusing syntax is avoidable.

Thanks Rash for the in-slot of comments on my code snippet! Let me response:

Having a variable named l is a bad idea.

I agree, but in such a small example where it is not hard to see the context, and within a "detail" class that shouldn't be looked at by users, I don't think it's that bad. But, yeah, it's a bad example to give.

Overloading the address-of operator is a terrible idea.

The alternatives are worse. The idea here is that the lockable variables act as a "normal" variable which can only be accessed by a pointer to it, it seems to make sense to use the address-of for that purpose. But your right, I should have used a function name like "lock()" instead.

Putting semicolons after function definitions is an idiosyncracy that you'll abandon someday.

That's just a reflex of mine. Sometimes it is necessary, mostly it is not, but who cares?

Defining the type as detail::locked_ptr_impl for no reason is another, it could have just been locked_ptr and you wouldn't have compatibility problems.

The reason for having locked_ptr and detail::locked_ptr_impl is because it uses the "scope-guard" technique. Look at it again, and you will see that those two need to be separate, because locked_ptr is not just a typedef for detail::locked_ptr_impl, it is a typedef for const detail::locked_ptr_impl& and that makes all the difference in the world.

swap(lockable<T>&, lockable<T>&) is broken.
It doesn't work properly when lhs and rhs are the same object. It tries to lock the mut field twice, but std::mutex is not recursively lockable.

You're right, I always forget that darn self-assignment / self-swap issue. I guess this would work:

// Swap function (only swaps the value, not the mutex):
friend void swap(lockable<T>& lhs, lockable<T>& rhs) {
  if(&lhs.value == &rhs.value)   // these are thread-safe reads (atomic).
    return;
  std::unique_lock<std::mutex> l1(lhs.mut), l2(rhs.mut);
  using namespace std;
  swap(lhs.value, rhs.value);
};

And I also agree that it is somewhat superfluous, as one should normally acquire a locked_ptr to each object and then swap them with T's swap function.

Also, I think that std::forward(arg) needs to be std::forward<Args>(arg), if you actually try to instantiate a lockable with constructor arguments. This might be only necessary in gcc 4.6.3.

Sorry, small typo, it should indeed be std::forward<Args>(arg). I guess it kind-of works regardless, but it is indeed a typo.

There's one example of a relatively innocuous-looking expression that will deadlock.

Definitely, my class is not entirely fool-proof, and never claimed it was. And, of course, it is not air-tight, one could easily grab a pointer to the mutex-protected value and annihilate the protection.

In this case of deadlocking, however, the solution is trivial. Replacing the mutex with a recursive_mutex seems to fix all those deadlocking problems you've exposed.

Also locked_ptr_impl(std::mutex& aMut, T* aPtr) is unnecessarily marked explicit.

Probably a remnant of an earlier version of the code. Sorry about that, I didn't notice it.

So, after all these constructive comments, I guess it calls for a version 2.0 of the code:

// lockable.hpp

/*
 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF
 * ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED
 * TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A
 * PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
 * SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR
 * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
 * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE
 * OR OTHER DEALINGS IN THE SOFTWARE.
 *
 * Author: Sven Mikael Persson
 * Date: June 27th, 2012
 * Modified: July 17th, 2012
 * Acknowledgements: Thanks to Rashakil Fol for pointing out several issues 
 *                   with the original code, especially the deadlocking issue.
 */

#ifndef LOCKABLE_OBJECT_HPP
#define LOCKABLE_OBJECT_HPP

#include <mutex>
#include <type_traits>

template<typename T>
class lockable;  // forward-declaration.

namespace detail {

  /*
   * This class template is a locked smart-pointer used under-the-hood.
   */
  template <typename T>
  class locked_ptr_impl {
    private:
      mutable std::unique_lock<std::recursive_mutex> mut_lock;
      T* value_ptr;

      // private constructor:
      locked_ptr_impl(std::unique_lock<std::recursive_mutex>&& aLock, T* aValuePtr) : 
                      mut_lock(std::move(aLock)), 
                      value_ptr(aValuePtr) { };

      // private move-constructor:
      locked_ptr_impl(locked_ptr_impl&& rhs) : 
                      mut_lock(std::move(rhs.mut_lock)), 
                      value_ptr(rhs.value_ptr) { };

      // private constructor:
      locked_ptr_impl(std::recursive_mutex& aMut, T* aValuePtr) : 
                      mut_lock(aMut), value_ptr(aValuePtr) { }; 
    public:
      locked_ptr_impl(const locked_ptr_impl& rhs) = delete;         //non-copyable
      locked_ptr_impl& operator=(const locked_ptr_impl&) = delete;  //non-copy-assignable
      locked_ptr_impl& operator=(locked_ptr_impl&&) = delete;       //non-move-assignable

      // Implicit conversion to the corresponding const smart-pointer:
      operator locked_ptr_impl<const T>() {
        return locked_ptr_impl<const T>(std::move(mut_lock), value_ptr); // <-- allowed by friendship.
      };

      // Friendship with non-const smart-pointer:
      friend class locked_ptr_impl< typename std::remove_const<T>::type >;
      // Friendship with the mutex wrapper class:
      friend class lockable<T>;

      // Dereference operator overload:
      T& operator*() const { return *value_ptr; };
      // Pointer-member access operator overload: 
      T* operator->() const { return value_ptr; }; 
  };

};

// Template alias for the public interface to the under-the-hood smart-pointer:
template <typename T>
using locked_ptr = const detail::locked_ptr_impl<T>&;

// For GCC version < 4.7.0, use this replacement for the template alias:
// template <typename T>
// struct locked_ptr {
//   typedef const detail::locked_ptr_impl<T>& type;
// };


/*
 * This class template is the main wrapper to associate a mutex to an object.
 */
template<typename T>
class lockable {
  private:
    mutable std::recursive_mutex mut;
    T value;
  public:
    // Forwarding constructor for ease of construction of the wrapped value:
    template <typename... Args>
    explicit lockable(Args&&... arg) : mut(), value(std::forward<Args>(arg)...) { }; 

    // Copy-constructor (copies only the value, generates a new mutex):
    lockable(const lockable<T>& rhs) : mut(), value(*(rhs.lock())) { };

    // Swap function (only swaps the value, not the mutex):
    friend void swap(lockable<T>& lhs, lockable<T>& rhs) {
      if(&lhs.value == &rhs.value)
        return;
      std::unique_lock<std::recursive_mutex> l1(lhs.mut), l2(rhs.mut);
      using std::swap;
      swap(lhs.value, rhs.value);
    };

    // Copy-and-swap (and, implicitly, move-and-swap):
    lockable<T>& operator=(lockable<T> rhs) { 
      swap(*this, rhs);
      return *this;
    };

    // Non-const address-of operator to obtain a locked pointer:
    detail::locked_ptr_impl<T> lock() { 
      return detail::locked_ptr_impl<T>(mut,&value);
    };

    // Const address-of operator to obtain a locked pointer:
    detail::locked_ptr_impl<const T> lock() const { 
      return detail::locked_ptr_impl<const T>(mut,&value);
    }; 
};


#endif


// test.cpp  (test program)

#include "lockable.hpp"

#include <iostream>
#include <string>
#include <thread>

lockable<std::string> my_str;     //create a lockable variable of type std::string.
std::string my_result_str;        //result of the test.

void printMyStr() {
  std::cout << *(my_str.lock()) + *(my_str.lock()) << std::endl; // no deadlock.
  swap(my_str,my_str);  // no deadlock.

  locked_ptr<const std::string> s = my_str.lock();  // locks the string.
  std::cout << *s << std::endl;               // prints its value.
  my_result_str += *s;                        // appends its value to the result.
};

int main() {
  std::thread* t;
  {
    locked_ptr<std::string> s = my_str.lock();  // locks the string.
    t = new std::thread(printMyStr);      // thread started, will be blocked until lock is released (end of scope).

    my_result_str = (*s = "Hello "); // set the first half of the message.
    std::cout << *s;                 // print the first half.
    *s = "World";                    // set the second half of the message.
  };                                 // release the lock on my_str (that should allow the other thread to unblock).
  t->join();                         // wait for the other thread to be finished.
  delete t;
  if(my_result_str == "Hello World") // check the result, should be "Hello World" if the threads were in sync.
    return 0;
  else
    return 1;  // error, my_result_str should be "Hello World" if the synchro worked correctly.
};

In such a small example where it is not hard to see the context, and within a "detail" class that shouldn't be looked at by users, I don't think it's that bad. But, yeah, it's a bad example to give.

It's bad because it's l which looks like 1, in particular, not just because it's a single letter.

The alternatives are worse.

I don't quite understand you because you then said that naming it lock would be better. It doesn't matter, the function really should not exist at all.

The reason for having locked_ptr and detail::locked_ptr_impl is because it uses the "scope-guard" technique. Look at it again, and you will see that those two need to be separate, because locked_ptr is not just a typedef for detail::locked_ptr_impl, it is a typedef for const detail::locked_ptr_impl& and that makes all the difference in the world.

Nobody expects a typedef to be a reference type, so you should never ever do that. For example, I didn't know that it was a reference type. As you can see, the code is confusing to the reader and too subtle. Abandoning the practice of having this method on lockable which obtains the locked pointer, and instead having a constructor on locked_ptr which takes a lockable<T>& argument, is more clear and more ordinary. That's why, for example, std::unique_lock<T> works that way, and why std::mutex::lock is a dumb method that returns void.

Also, even if we wanted to keep the interface the way it was, you just need to make locked_ptr's move constructor public, and then you don't need the typedef to be a const reference.

Definitely, my class is not entirely fool-proof, and never claimed it was. And, of course, it is not air-tight, one could easily grab a pointer to the mutex-protected value and annihilate the protection.

Those are different kinds of fool-proof. The behavior of swap is surprising, it acquires a lock on something without the user knowing. You just don't do that.

In this case of deadlocking, however, the solution is trivial. Replacing the mutex with a recursive_mutex seems to fix all those deadlocking problems you've exposed.

It doesn't solve the swap deadlocking surprise at all.

One reason why being a const reference can kill you is that somebody could easily unknowingly initialize another const reference that escapes the lifetime of the locked_ptr object. One person might write locked_ptr<std::string::type u = s;, the next person sees that and thinks locked_ptr uses a shared lock, because that's what it looks like. And then they write something like f = new foo(s) and you get a possibly unnoticeable memory corruption bug.

Nobody expects a typedef to be a reference type, so you should never ever do that.

Making a const-reference or reference typedef is a fairly common thing. I don't think the "nobody expects" and the "never ever do that" are appropriate (if ever).

With that said, I do understand your concern that the const-reference typedef might make it look like not a reference. That could be solved by just not having the typedef and instead forcing the user to do something like const locked_ptr& s = my_str.lock(); instead. It's a tradeoff between syntax sugar and explicitness (if that's even a word). That typedef in this particular case might not be appropriate, but it's a debatable issue, and somewhat a matter of preference too.

instead having a constructor on locked_ptr which takes a lockable<T>& argument, is more clear and more ordinary. That's why, for example, std::unique_lock<T> works that way, and why std::mutex::lock is a dumb method that returns void.
Also, even if we wanted to keep the interface the way it was, you just need to make locked_ptr's move constructor public, and then you don't need the typedef to be a const reference.

That is certainly a viable alternative, but I still prefer my way. Because the purpose of my locked_ptr is to obtain a lock within a function body (or inner-scope within a function) and restrict the locking to that function body only, and not allow any other uses of locked_ptr. A unique_lock (or unique_ptr) type of pattern does not achieve that, comes close, but not all the way. My solution does not go all the way either as it is possible to hack your way out of it, but it is a step closer.

The behavior of swap is surprising, it acquires a lock on something without the user knowing. You just don't do that.

The behavior of the swap is not any more surprising than the behavior of the lockable<T> type itself. The lockable is an object whose every single accesses and mutations are mutex-protected. Why would anyone expect that swapping two lockable objects wouldn't be mutex-protected as well? Except for the deadlock problem, which is now solved, I don't see any problems with the swap function.

It doesn't solve the swap deadlocking surprise at all.

Huhh? I don't know about the "surprise" because I don't know what is surprising about it, but it certainly solves the deadlocking problem, as seen in my second version which calls swap(my_str,my_str); without any deadlock.

The article starter has earned a lot of community kudos, and such articles offer a bounty for quality replies.