Re: Upcomming Krusader-1.80.0-beta1 and Debian bug #403130, ftp problem

[Karai Csaba]

I reproduced the bug. Unfortunately, it won't be easy to fix.

I'll shortly tell what's happening:

1. create 2 tabs and open 2 slow remote connection in them (dir listing
is slow)
2. on tab1 refresh (Ctrl+R)
3. on tab2 refresh paralelly (tab1 is refreshing)!!! (Ctrl+R)

What's happening?
- Tab1:
qApp->processEvents() is called and waiting to finish
- Tab2:
inside the Tab1 cycle another qApp->processEvents() is called

I think, that Krusader don't really like embedded qApp->processEvents()
calls.

My personal suggestion is to make a hard work before 1.80 and REMOVE ALL
qApp->processEvents() calls. That will drastically increase the
stability of Krusader.

I think, that 90% of the Krusader crashes are caused by calling
qApp->processEvents().

Rafal, Fathi:
-----------------------------------

Avoiding the crash: please don't do anything with krusader, while a tab
is refreshing. Wait the end of the refresh, and after that it's safe to
work. Before 1.80 we will find a solution to the problem.

Thanks,

Csaba

[Jiri Palecek]

Hello

Karai Csaba wrote:
> I reproduced the bug. Unfortunately, it won't be easy to fix.
>
> I'll shortly tell what's happening:
>
> 1. create 2 tabs and open 2 slow remote connection in them (dir listing
> is slow)
> 2. on tab1 refresh (Ctrl+R)
> 3. on tab2 refresh paralelly (tab1 is refreshing)!!! (Ctrl+R)
>
> What's happening?
> - Tab1:
> qApp->processEvents() is called and waiting to finish
> - Tab2:
> inside the Tab1 cycle another qApp->processEvents() is called

Actually, this would not cause a crash, only a freeze (till both are
refreshed).
But yes, this is where the crashes come from. A much more pathological
situation is when yo eg. copy something into a panel while in refresh,
or if you refresh while in refresh.

> I think, that Krusader don't really like embedded qApp->processEvents()
> calls.

Nobody likes them :-)

> My personal suggestion is to make a hard work before 1.80 and REMOVE ALL
> qApp->processEvents() calls. That will drastically increase the
> stability of Krusader.

I don't think it is necessary, although it would be really nice.

> I think, that 90% of the Krusader crashes are caused by calling
> qApp->processEvents().

Regards
Jiri Palecek

[Jiri Palecek]

BTW, could you (or someone), please, try if this version crashes too?

http://www.ms.mff.cuni.cz/~palej3am/krusader_kde3_new.tar.bz2

[Shie Erlich]

On 1/3/07, Jiri Palecek <jpal...@web.de> wrote:

> My personal suggestion is to make a hard work before 1.80 and REMOVE ALL
> qApp->processEvents() calls. That will drastically increase the
> stability of Krusader.

I don't think it is necessary, although it would be really nice.

question: iirc, most processEvents calls are used as a way of letting Qt do
the thing it needs - and basically, let the event loop run its course.
if we remove those calls, we will have to either leave the function we're in,
or we'll be blocking the event loop.
am i missing something ?

--
Shie Erlich
http://www.krusader.org/

[Karai Csaba]

> question: iirc, most processEvents calls are used as a way of letting Qt do
> the thing it needs - and basically, let the event loop run its course.
> if we remove those calls, we will have to either leave the function
> we're in,
> or we'll be blocking the event loop.
> am i missing something ?

Yes. Start a new thread and this thread just calls usleep (like calc
space), while the main thread calls qApp->processEvents()

The best solution is different in each case:

- for the panels: leaving the function would be the best
- searcher / packer: separate thread

Unfortunately it would be a big work, so let's postpone it as a beta is
out. I'll check how we could avoid crash without rewriting all.

Thanks,

Csaba

[Jonas Bähr]

Am 09.01.2007 um 21:41 schrieb Karai Csaba:
>
>> question: iirc, most processEvents calls are used as a way of
>> letting Qt do
>> the thing it needs - and basically, let the event loop run its
>> course.
>> if we remove those calls, we will have to either leave the function
>> we're in,
>> or we'll be blocking the event loop.
>> am i missing something ?
>
> Yes. Start a new thread and this thread just calls usleep (like calc
> space), while the main thread calls qApp->processEvents()

you may want to search kde's mailinglists (kde-devel or kde-core-
devel). There was some time ago a big discussion about the pros and
cons of qApp->processEvents() and alternatives. IIRC the chorus was
not to use processEvents but install an eventFilter and process only
filtered events...

bye,
Jonas

[Karai Csaba]

Unfortunately we cannot escape from removing qApp->processEvents() from
vfs_refresh().

Situation:
============
The FTP server is slow
... You are in vfs_refresh() for 5s

During that time the user can the press refresh button again!!!
The call stack:
...
vfs_refresh()
...
vfs_refresh()
...

When vfs_refresh() finishes -> immediate crash!!!

The vfs_filesP pointer is even refers to the temporary content, so
searching and anything else in VFS will not be available...
The content of vfs is absolutely inconsistent in that time!

The user could press any other buttons, that may call vfs_refresh(),
such as '/' or '..' or can copy files to the refreshing folder.

Recursive vfs_refresh() will always cause crash...

My idea:
=========

Logically the best is splitting vfs_refresh() to

vfs_startRefresh()
and vfs_refreshFinished() signal

vfs_startRefresh will do nothing with the current view.
As soon as all data arrives, the view will be refreshed to the
new content. That keeps the view always consistent.

Csaba

Karai Csaba írta: