Glib::Threads::Private wraps GPrivate, which seems to wrap pthread_setspecific() and pthread_getspecific():
https://developer.gnome.org/glib/stable/glib-Threads.html#GPrivate

Maybe we should generally discourage use of any pthread API at the same time as the higher-level C++ concurrency API?

Created attachment 314972 [details] [review]
0001-tests-glibmm_mainloop-Use-the-std-thread-API-instead.patch

Created attachment 314973 [details] [review]
0001-Network-examples-Use-std-thread-instead-of-Glib-Thre.patch

Created attachment 314980 [details] [review]
0001-examples-dispatcher-Use-std-thread-and-friends.patch

Created attachment 314983 [details] [review]
0001-thread-example-Use-std-thread-etc-instead-of-depreca.patch

For this one, I added some unlock() calls before the condition_variable::notify_one() calls, even though there were no corresponding Glib::Threads::Lock::release() calls to replace.

Created attachment 314984 [details] [review]
0001-threadpool-example-Use-std-mutex-instead-of-Glib-Thr.patch

Glib::ThreadPool doesn't use the Glib::Threads::* API but I wonder if we should deprecate it anyway.

There seems to be some confusion about whether a thread pool is necessary with C++11 std::thread, or whether that is implementation dependent. Anyway, we would be encouraging mixing of 2 thread APIs if we left it undeprecated.

Created attachment 314985 [details] [review]
0005-Threads-Deprecate-in-favour-of-the-C-11-C-14-concurr.patch

This one mentions std::lock_guard and (std::unique_lock) instead of std::lock.

It would be nice if people could check my patches that update the tests and examples. We could then push them already without waiting for the Glib::Threads deprecation.

Review of attachment 314983 [details] [review]:

::: examples/thread/thread.cc
@@ +112,3 @@
   MessageQueue queue;
 
+  auto *const producer = new std::thread(

std::unique_ptr would be nicer here. And since we'll be turning on c++14 you can std::make_unique

examples/network
  resolver.cc:
      G_LOCK_DEFINE_STATIC (response);
    can be replaced by
      static std::mutex response;

  socket-client.cc and socket-server.cc:
    //TODO: This won't happen if we returned earlier.
    is a note that something remains to be done with std::thread* thread.
    A have no detailed solution now. Perhaps replace the dumb pointer by
      std::unique_ptr<std::thread, join_and_delete> thread;
      //...
      void join_and_delete(std::thread* thread)
      {
        thread->join();
        delete thread;
      }

examples/thread/dispatcher
examples/thread/dispatcher2
  OK

examples/thread/thread
  The main function can be simplified to
    int main(int, char**)
    {
      Glib::init();

      MessageQueue queue;

      std::thread producer(&MessageQueue::producer, &queue);
      std::thread consumer(&MessageQueue::consumer, &queue);

      producer.join();
      consumer.join();

      std::cout << std::endl;

      return 0;
    }
  Well, the replacement of the lambda expressions is perhaps not an obvious
  simplification, but allocating the threads on the stack is.

examples/thread/threadpool
  OK

tests/glibmm_mainloop
  second_thread can be allocated on the stack:
    std::thread second_thread(
      [first_thread_id, first_mainloop]
      {
        thread_function(first_thread_id, first_mainloop);
      });
    // ...
    second_thread.join();

  An alternative to the lambda expression:
    std::thread second_thread(thread_function, first_thread_id, first_mainloop);


>>>Comments to the patch that deprecates Glib::Threads

When all of glib/src/threads.hg is deprecated, it should be moved to
glibmm_files_deprecated_hg in glib/src/filelist.am.

When everything in the Glib::Threads namespace is deprecated, it should not be
used in glibmm. But it is. Deprecated stuff is used in

  glib/glibmm/dispatcher.cc
    static Threads::Private<> Glib::DispatchNotifier::thread_specific_instance_
    Threads::Mutex Glib::DispatchNotifier::mutex_ // ifdef G_OS_WIN32

  glib/glibmm/exceptionhandler.cc
    static Glib::Threads::Private<HandlerList> thread_specific_handler_list

  glib/glibmm/main.[h|cc]
  glib/glibmm/main.cc
    Glib::MainContext::wait(Glib::Threads::Cond&, Glib::Threads::Mutex&)

  glib/glibmm/main.cc
    Glib::Threads::Mutex extra_source_data_mutex;

  glib/glibmm/objectbase.[h|cc]
  glib/glibmm/object.cc
  glib/glibmm/interface.cc
    static Threads::Mutex* Glib::ObjectBase::extra_object_base_data_mutex

  glib/glibmm/threadpool.cc
    Glib::Threads::Mutex Glib::ThreadPool::SlotList::mutex_
    catch(Glib::Threads::Thread::Exit&)

  gio/src/application.ccg
    Glib::Threads::Mutex option_arg_callback_data_mutex

(In reply to Kjell Ahlstedt from comment #10)

>   glib/glibmm/main.[h|cc]
>   glib/glibmm/main.cc
>     Glib::MainContext::wait(Glib::Threads::Cond&, Glib::Threads::Mutex&)

That wraps g_main_context_wait(). I cannot think of any way to replace our wait() with a wait() that uses std::condition_variable, without reimplementing g_main_context_wait or, maybe notifying a std::condition_variable when an internal GCond is notified, which sounds a bit unpleasant.

We could say "Use the C API if you really want this". It doesn't seem really generally useful, right?

>   glib/glibmm/objectbase.[h|cc]
>   glib/glibmm/object.cc
>   glib/glibmm/interface.cc
>     static Threads::Mutex* Glib::ObjectBase::extra_object_base_data_mutex

I wonder if it would break ABI to change that static Mutex* (as in, exported to other object files) to a std::mutex*. It's meant to be private API anyway, right? Or is it used outside of glibmm?

(In reply to Murray Cumming from comment #11)
> >     Glib::MainContext::wait(Glib::Threads::Cond&, Glib::Threads::Mutex&)
>
> We could say "Use the C API if you really want this". It doesn't seem really
> generally useful, right?

I don't know if it's useful. It doesn't look very useful to me. And I realize
that it would be difficult to replace Glib::Threads::Cond and Glib::Threads::Mutex
by std::condition_variable and std::mutex here. So yes, it's probably best to
deprecate Glib::MainContext::wait(), direct app programmers to the C API, and
see if anyone complains.

> >   glib/glibmm/objectbase.[h|cc]
> >   glib/glibmm/object.cc
> >   glib/glibmm/interface.cc
> >     static Threads::Mutex* Glib::ObjectBase::extra_object_base_data_mutex
>
> I wonder if it would break ABI to change that static Mutex* (as in, exported
> to other object files) to a std::mutex*. It's meant to be private API
> anyway, right? Or is it used outside of glibmm?

It shall not be used in any class except ObjectBase, Object and Interface.
It's part of a workaround for added instance data in ObjectBase, which we can add
only at the next ABI break. It was changed from std::auto_ptr<Threads::Mutex> to
Threads::Mutex* as a fix for bug 748630. Now it can be changed to
std::unique_ptr<std::mutex> or even to std::mutex. The reason why it's a pointer
is explained in objectbase.h. That reason does not apply, if it's replaced by
a std::mutex.

OK. I've pushed all the patches to glibmm's master and I think I've addressed all the suggestions in subsequent commits. Thanks.

Deprecated classes (Threads::Private, Threads::Mutex, Threads::Thread::Exit)
are still used in several glib/glibmm/*.cc files and in gio/src/application.ccg.
Are they deliberately not replaced?

No, that wasn't on purpose. I've just replaced the use of Glib::Threads::* in giomm' application.cc glibmm's main.c.

I have not done anything about the use of Glib::Threads::Private<> in dispatcher.cc and exceptionhandler.cc. I wonder if we can replace Private with the thread_local keyword in C++11:
http://en.cppreference.com/w/cpp/language/storage_duration

Created attachment 316366 [details] [review]
0001-Use-thread_local-instead-of-Glib-Threads-Private.patch

Hopefully it's this easy to replace Glib::Threads::Private<> with thread_local, but it would be nice if someone more familiar with it could check this patch, please.

I have fixed a bug that crept into the code when Glib::Threads::Mutex::Lock was
replaced by std::unique_lock. Glib::Threads::Mutex::Lock::release() must be
replaced by std::unique_lock::unlock().
https://git.gnome.org/browse/glibmm/commit/?id=c7d7d5f169103b2d16527c52a95a5dc78e216da6

Then I tested the patch that replaces Glib::Threads::Private by thread_local.
The test cases that use Glib::Dispatcher (glibmm/examples/thread/dispatcher.cc,
dispatcher2.cc and gtkmm-documentation/examples/book/multithread) all work as
expected. I just changed
  // This causes deletion of the notifier object.
  thread_specific_instance_ = nullptr;
to
  delete instance;
  thread_specific_instance_ = nullptr;

Thanks. That all now in master and I've release a tarball.