[interfacekit] deadlock with BApplication/BWindow

Hi guys,


    Have a poroblem when quiting be_app. I have a solution for it but I 
want your opinion on it.

The problem:
------------------------------------------
bool
BApplication::QuitRequested()
{
        return quit_all_windows(false);
}
bool
BApplication::quit_all_windows(bool force)
{
        AssertLocked();

        if (window_quit_loop(false, force))
                return true;

        return window_quit_loop(true, force);
}
bool
BApplication::window_quit_loop(bool quitFilePanels, bool force)
{
        BList looperList;
>>>>>>>>BObjectLocker<BLooperList> listLock(gLooperList);
        if (listLock.IsLocked())
                gLooperList.GetLooperList(&looperList);

        for (int32 i = looperList.CountItems(); i-- > 0; ) {
                BWindow *window = dynamic_cast<BWindow *>((BLooper 
*)looperList.ItemAt(i));

                // ToDo: windows in this list may already have been closed in 
the mean time?!

                if (window != NULL && window->Lock()) {
                        if ((window->IsFilePanel() && !quitFilePanels)
                                || (!force && !window->QuitRequested())) {
                                // the window does not want to quit, so we 
don't either
                                window->Unlock();
                                return false;
                        }

>>>>>>>>>>>            window->Quit();
                }
        }

        return true;
}

void BWindow::Quit(){

...
        BLooper::Quit();
}
void BLooper::Quit()
{
        else
        {
PRINT(("  Run() has already been called and we are not the looper thread\n"));

                BMessage message(_QUIT_);
                message.AddInt32("testfield", 42);
                err = PostMessage(&message);

                // Also as per the BeBook, we have to wait until the looper is 
done
                // processing any remaining messages.
                int32 temp;
                do
                {
PRINT(("  wait_for_thread(%ld)...\n", tid));
                        err = wait_for_thread(tid, &temp);
                } while (err == B_INTERRUPTED);
        }
}
------------------------------------------

in BWindow's task_looper() after _QUIT_ is received, Lock() is called:

------------------------------------------
bool BLooper::Lock()
{
        // Defer to global _Lock(); see notes there
        return _Lock(this, -1, B_INFINITE_TIMEOUT) == B_OK;
}
status_t BLooper::_Lock(BLooper* loop, port_id port, bigtime_t timeout)
{
...
/**
        @note   We lock the looper list at the start of the lock operation to
                        prevent the looper getting removed from the list while 
we're
                        doing list operations.  Also ensures that the looper 
doesn't
                        get deleted here (since ~BLooper() has to lock the list 
as
                        well to remove itself).
 */
        {
>>>>>>>>>>>>        BObjectLocker<BLooperList> ListLock(gLooperList);
...

                // sLooperListLock automatically released here
        }
...
}
------------------------------------------

oups, deadlock! :-)

Here's my solution:

BApplication::window_quit_loop(bool quitFilePanels, bool force)
{
        BList looperList;
        int32 i;
        BWindow *window;
        bool moreWindows = true;

        while (moreWindows)
        {
                gLooperList.Lock();
                if (gLooperList.IsLocked())
                {
                        moreWindows = false;

                        looperList.MakeEmpty();
                        gLooperList.GetLooperList(&looperList);
                        for (i = looperList.CountItems(); i-- > 0; )
                        {
                                window = dynamic_cast<BWindow *>((BLooper 
*)looperList.ItemAt(i));
                                if (window != NULL && window->Lock()) {
                                        moreWindows = true;
                                        if ((window->IsFilePanel() && 
!quitFilePanels)
                                                || (!force && 
!window->QuitRequested())) {
                                                // the window does not want to 
quit, so we don't either
                                                window->Unlock();
                                                return false;
                                        }

                                        // 'window' lock is acquired.
                                        // TODO: still, is this safe? can 
'window' be invalid?
                                        gLooperList.Unlock();

                                        window->Quit();

                                        // go for 'while'.
                                        // a window in current list may already 
be invalid.
                                        break;
                                }
                        }
                        if (!moreWindows)
                                gLooperList.Unlock();
                }
                else
                        return false;
        }

        return true;
}

That's it. Comments, other ideas?


Also, why BLooperList::GetLooperList() returns NULL(removed) loopers?

void BLooperList::GetLooperList(BList* list)
{
        BAutolock Listlock(fLock);
        AssertLocked();
        for (uint32 i = 0; i < fData.size(); ++i)
        {
// INSERTED by me: if (fData[i].looper)
                        list->AddItem(fData[i].looper);
        }
}


Thanks,
Adi.

Other related posts: