Le 20/01/2017 à 11:14, Olivier Goffart a écrit :
On Dienstag, 17. Januar 2017 11:21:56 CET Grégoire Barbier wrote:
Le 16/01/2017 à 10:34, Olivier Goffart a écrit :
> What's the use case for this function? For direct call you better of
> calling the function directly,  and the equivalent of QueuedConnection
> can be achieved with QTimer::singleShot.

Hi.

AFAIK there is no other way to call a method across threads *and wait
for it result* than QMetaObject::invokeMethod() with
Qt::BlockingQueuedConnection and Q_RETURN_ARG (apart, of course, making
the called method thread-safe).

That's a valid use case. Thank you for bringing that up.

[...]

Also (I still dream), if there was a way to make invokeMethod()
automagically choose between a direct call and
Qt::BlockingQueuedConnection, it would be possible to get rid of this idiom:
if (QThread::currentThread() == this->thread())
        foo = func();
     else
        QMetaObject::invokeMethod(this, "func",
                  Qt::BlockingQueuedConnection,
                  Q_RETURN_ARG(foo));

Kind of Qt::DirectOrBlockingQueuedConnection.

See the discussion in https://codereview.qt-project.org/83404/ for why this is
not a good idea.
In summary, BlockingQueuedConnection is dangerous as it can lead to deadlock
if the other thread is waiting on your thread. In order to use it properly,
you really need to know, while programming in which thread you are and which
thread you try to communicate to, and be sure that there is never in your
program cases where the other thread may be waiting on a QWaitCondition for
your other thread. So this pattern with QThread::currentThread() == this-
thread() before a BlockingQueuedConnection is really dangerous.

Thank you it's very interesting.

But, I use this pattern mainly to protect data-access methods (such as a getter on an implicitly shared object) than can be either called from the owner thread or another one. I'll keep using it, without fear, unless the called object's owner thread was likelly to change.

I respect the fact that you rejected Qt::BlockingAutoConnection in 2014 because of its potential danger, but I'm not sure that it's better to let people use the "QThread::currentThread() == this" pattern without being warned rather than implementing Qt::BlockingAutoConnection, with a detailed warning in the doc.

Moreover, I don't understand whi BlockingAutoConnection would be more dangerous (if it were to be added) than BlockingQueuedConnection is currently already dangerous.

With your own words from 2014:

"""
I would not even try to solve this race with moveToThead, as a mention, i think we just need to document that one must not call moveToThread when Blocking connecitona re involved.
"""

Maybe it would be interesting to set this comment in enum Qt::ConnectionType doc, just right to Qt::BlockingQueuedConnection (or a would-be Qt::BlockingAutoConnection) and not only in the "Signals and Slots Across Threads" paragraph, to make everything safer with noobs (like me) that do not read the whole docs but only spam the F1 key from with Qt Creator.

On the other hand, it would be useful to get the return value of a
QueuedConnection in a QFuture.

Well.
In french there is a very funny expression which means more or less "to use a hammer to kill a fly". ;-) Let's take an example, I clearly don't think I would like to use a QFuture, make it thread-safe with a QMutex and the like in the following code (even though you've just make me learn that Qt::BlockingQueuedConnection may be dangerous, in the following code I know it's not, because A object never changes thread):

class B;
class A : public QObject {
  Q_OBJECT
  QLinkedList<B> _queue;
public:
  /** This method is thread-safe. */
  B pop() const {
    B value;
    if (QThread::currentThread() == this->thread())
      value = doPop();
    else
      QMetaObject::invokeMethod(this, "doFoo",
             Qt::BlockingQueuedConnection,
             Q_RETURN_ARG(B, value));
    return value;
  }
private:
  /** This method is not thread-safe. */
  Q_INVOKABLE B doPop() {
    return _queue.takeFirst();
  }
};

In the other hand, I would by far prefer to write this code, both the lambda and Qt::AutoBlockingConnection make the code shorter and easier to understand (and I don't expect that using QFuture would lead to the same result):

class B;
class A : public QObject {
  Q_OBJECT
  QLinkedList<B> _queue;
public:
  /** This method is thread-safe. */
  B pop() const {
     B value;
     QMetaObject::invokeMethod(this, [&value]() {
             value = _queue.takeFirst();
         }, Qt::AutoBlockingConnection);
     }
  }
};

Of course in this simple case, it's obvious that make the method thread-safe (e.g. with a QMutex/QMutexLocker) is easier and lighter than a queued call. But it's not always so obvious.

--
Grégoire Barbier :: g à g76r.eu :: +33 6 21 35 73 49
_______________________________________________
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Reply via email to