[Qt] Удаление объектов и сигналы

QPointer guard( this );
emit someSignal();
if ( !guard )
    return;

Daniel to qt-bugs
	
show details 12/28/08
	
Hello Trolltech hackers!

I have trapped into dead lock in my program because of
Qt::BlockingQueuedConnection signal emitting. I know about awares of
usage this type of connection, so I have rechecked Qt docs as for this
and think this is a Qt bug.

So how it works.

I have worker object in separate thread, that emits signal with
Qt::BlockingQueuedConnection and waits until slot in main thread not
finished. Fine.
Main thread simultaneously decides to destroy owner of this slot and
that worker object in the separate thread. Okay, this is a normal
cleanup.

In the main thread I don't know if worker object already emitted
signal and already waits right now. Also I can't just delete him
because: 1) it in the other thread 2) it could be in the middle of
emitting.

So what should I do in destructor of object in main thread?

0. Optionally. Mark worker object as aborted to finish it's body as
fast as possible.
1. Call worker->disconnect(this) to ensure that even worker had
emitted blocking signal - I will not catch it.
2. Wait until thread finished.

But this not works because QObject::disconnect() not checks for the
current pending emitted signals. So worker object just stops at
internal QWaitCondition instance and waits forever.

My opinion:
QObject::disconnect() must check not only connection links from sender
to receiver - but also pending but not yet delivered
Qt::BlockingQueuedConnection and wake that QWaitCondition.

What do you think?

Thanks!

Daniel to qt-bugs

Forgot to said.

For now I am using following workaround for this issue:

...

void MyRunnable::run()
{
 emit finished();
}

...

void ObjectFromMainThread::finishedSlot()
{
 // workaround code
 // if no sender than myRunnable->disconnect() has been called
 // ignore this signal
 if ( !sender() )
   return;
 ...
}

ObjectFromMainThread::~ObjectFromMainThread()
{
 // cleaning up
 foreach ( MyRunnable * myRunnable, myRunnables_ )
 {
   myRunnable->isAborted = true; // just to speed up useless work
   myRunnable->disconnect();
 }

 // if some of runnables emitts signal now - we will catch it in
finishedSlot() with NULL sender
 QCoreApplication::sendPostedEvents( this, QEvent::MetaCall );

 myThreadPool_->waitForDone(); // if pool was used. Or
myThread_->wait() in case of QThread.
}


So my suggestion - is to avoid checking for sender() in slot and omit
QCoreApplication::sendPostedEvents( this, QEvent::MetaCall ). Both are
non-obvious and useless from my point of view.

(-:

qt-bugs@trolltech.com
 to me
	
show details 1/6/09
	
Daniel,
- Show quoted text -

On Sunday, 28. Dec 2008 07:14 Daniel Levin wrote:
> I have trapped into dead lock in my program because of
> Qt::BlockingQueuedConnection signal emitting. I know about awares of
> usage this type of connection, so I have rechecked Qt docs as for this
> and think this is a Qt bug.
>
> So how it works.
>
> I have worker object in separate thread, that emits signal with
> Qt::BlockingQueuedConnection and waits until slot in main thread not
> finished. Fine.
> Main thread simultaneously decides to destroy owner of this slot and
> that worker object in the separate thread. Okay, this is a normal
> cleanup.
>
> In the main thread I don't know if worker object already emitted
> signal and already waits right now. Also I can't just delete him
> because: 1) it in the other thread 2) it could be in the middle of
> emitting.
>
> So what should I do in destructor of object in main thread?
>
> 0. Optionally. Mark worker object as aborted to finish it's body as
> fast as possible.
> 1. Call worker->disconnect(this) to ensure that even worker had
> emitted blocking signal - I will not catch it.
> 2. Wait until thread finished.
>
> But this not works because QObject::disconnect() not checks for the
> current pending emitted signals. So worker object just stops at
> internal QWaitCondition instance and waits forever.
>
> My opinion:
> QObject::disconnect() must check not only connection links from sender
> to receiver - but also pending but not yet delivered
> Qt::BlockingQueuedConnection and wake that QWaitCondition.
>
> What do you think?

This is actually supposed to work already but there could be a bug
somewhere of course. What should be happening in this case is that for
every blocking queued connection there is a queued MetaCall event for
the receiver. And when the receiver is deleted, all pending posted
events are also deleted. And the QMetaCallEvent destructor will release
the semaphore that the sender is blocked on and thus the sender thread
should wake up.

Could you possibly send us an example that shows the problem?

Thanks.

Daniel to qt-bugs
	
show details 1/6/09
	
Attached example.

This will trap into deadlock:
./run

This will use workaround to not trap:
./run w

Some comments inside sources.

qobject_disconnect.tar.gz

qt-bugs@trolltech.com
 to me
	
show details 1/8/09
	
Daniel,

On Tuesday, 06. Jan 2009 15:37 Daniel Levin wrote:
> Attached example.
>
> This will trap into deadlock:
> ./run
>
> This will use workaround to not trap:
> ./run w
>
> Some comments inside sources.

Indeed, the program shows that QObject::disconnect() doesn't enforce
that the slot is invoked, but I'm not sure it should. The disconnect()
call will just enforce that the next emit does not result in the slot
being invoked again. At this point in your program the signal has
already been emitted and is just waiting for the slot to be invoked.
And that one will only be invoked when the event loop starts spinning
again (or if you explicitly enforce that the MetaCall events are
processed).

However, the problem you outlined in your initial mail should be working
even if disconnect() doesn't process these events, unless you are doing
something similar there as well (i.e. blocking the main thread until
the spawned thread has finished)?

Daniel to qt-bugs
	
show details 1/11/09
	
Hello again!

Sorry for late answer.

So the question is what disconnect() method is for.
The name itself slitely confuses.
From one point of view it states for low-level breaking link between
sender and receiver.

But from developer point of view disconnect means other.
It declares use-case rule of usage connect()/disconnect() pair:

1. After calling connect() receiver object will receive signals from sender.
2. After calling disconnect() receiver object will not receive signals
from sender.

Simple.
Thats use-case and thats what developer expects.
Breaking link between sender and receiver is just an internal implementation.
Anyway, it is good to understand how it works from Qt documentation,
but actual purpose of disconnect() is to declare use-case, not to
describe how internal processed are organized.

This is my opinion and I think many Qt developers already bases on this rule.

Current implementation not follows this use-case, what leads to
unexpected behavior:
1. Receiving zero sender() in slot, connected with
BlockingQueuedConnection, which guaranties that sender() always
present.
2. Pending signal that should be cleaned up with processing MetaCall events.

I understand that backward compatibility is very important. But in
this case I don't think that cleaning posted MetaCall events for
disconnected signals can lead to issues.

So to be completely clear what I mean. If I would had a chance to
design behavior of disconnect() method - documentation for it would
was something like:
"Disconnects signal in object sender from method in object receiver.
Receiver object will not receive the given signal into specified
method from sender after this call."

qt-bugs@trolltech.com
 to me
	
show details 1/19/09
	
Daniel,
- Show quoted text -

On Sunday, 11. Jan 2009 15:43 Daniel Levin wrote:
> So the question is what disconnect() method is for.
> The name itself slitely confuses.
> From one point of view it states for low-level breaking link between
> sender and receiver.
>
> But from developer point of view disconnect means other.
> It declares use-case rule of usage connect()/disconnect() pair:
>
> 1. After calling connect() receiver object will receive signals from
> sender.
> 2. After calling disconnect() receiver object will not receive signals
> from sender.
>
> Simple.
> Thats use-case and thats what developer expects.
> Breaking link between sender and receiver is just an internal
> implementation.
> Anyway, it is good to understand how it works from Qt documentation,
> but actual purpose of disconnect() is to declare use-case, not to
> describe how internal processed are organized.
>
> This is my opinion and I think many Qt developers already bases on
> this rule.
>
> Current implementation not follows this use-case, what leads to
> unexpected behavior:
> 1. Receiving zero sender() in slot, connected with
> BlockingQueuedConnection, which guaranties that sender() always
> present.
> 2. Pending signal that should be cleaned up with processing MetaCall
> events.
>
> I understand that backward compatibility is very important. But in
> this case I don't think that cleaning posted MetaCall events for
> disconnected signals can lead to issues.

Probably not but even if we would add this there wouldn't be
consistency. For instance, what would happen if you disconnect your
thread just after emitting a non-blocking signal? This disconnect
statement can't process the meta call events since they belong to a
different thread and have to be processed there.

> So to be completely clear what I mean. If I would had a chance to
> design behavior of disconnect() method - documentation for it would
> was something like:
> "Disconnects signal in object sender from method in object receiver.
> Receiver object will not receive the given signal into specified
> method from sender after this call."

I agree in principle but it's not possible to catch all corner cases I'm
afraid. I'll notify our doc team about this though and see if the
documentation can be clarified in this respect.

Daniel to qt-bugs
	
show details 1/19/09
	
>
> Probably not but even if we would add this there wouldn't be
> consistency. For instance, what would happen if you disconnect your
> thread just after emitting a non-blocking signal? This disconnect
> statement can't process the meta call events since they belong to a
> different thread and have to be processed there.
>

This is really thin moment what use-case should be when disconnect()
body executes not in the receiver thread.
We defenitely can't interact with currently processed signal in that thread.
But I don't see obstacles to clear pending meta call events for this
broken connection in the separate thread.

Just recall that I have implemented similar signal/slot behavior on
non-Qt'able platform.
In that case separate thread took copy of pending events and guarded
them with mutexes so any underlying disconnection was able to access
this list of pending events and clean them.

qt-bugs@trolltech.com
 to me
	
show details 1/21/09
	
Daniel,

On Monday, 19. Jan 2009 16:28 Daniel Levin wrote:
> > Probably not but even if we would add this there wouldn't be
> > consistency. For instance, what would happen if you disconnect your
> > thread just after emitting a non-blocking signal? This disconnect
> > statement can't process the meta call events since they belong to a
> > different thread and have to be processed there.
>
> This is really thin moment what use-case should be when disconnect()
> body executes not in the receiver thread.
> We defenitely can't interact with currently processed signal in that
> thread.
> But I don't see obstacles to clear pending meta call events for this
> broken connection in the separate thread.

I can't see any either but that doesn't mean there aren't any. Either
way, this won't be consistent.

I'll discuss this some more with our object model people but most likely
this won't be changed and I'd recommend you keep calling
sendPostedEvents() as you did in the code you sent us earlier.

Daniel to qt-bugs
	
show details 1/21/09
	
Okay, thanks for helping, Erik!