448784 – ScrollBar binding loop can cause apps to freeze

Bug 448784 - ScrollBar binding loop can cause apps to freeze

Summary: ScrollBar binding loop can cause apps to freeze

Status:	RESOLVED FIXED

Alias:	None

Product:	frameworks-qqc2-desktop-style
Classification:	Frameworks and Libraries
Component:	general (show other bugs)
Version:	5.90.0
Platform:	Compiled Sources Linux

Importance:	HI crash
Target Milestone:	---
Assignee:	Marco Martin

URL:
Keywords:

Duplicates (5):	401533 446934 447080 449581 451007 (view as bug list)
Depends on:
Blocks:

Reported:	2022-01-19 17:28 UTC by Matej Starc
Modified:	2022-07-14 13:12 UTC (History)
CC List:	17 users (show)

See Also:
Latest Commit:	https://invent.kde.org/frameworks/kirigami/commit/c28dff79721d3805edcd413ef233216b3b3b0104
Version Fixed In:	5.97
Sentry Crash Report:

Attachments
how to reproduce video (3.34 MB, video/x-matroska) 2022-01-19 17:28 UTC, Matej Starc	Details
View All Add an attachment

Note You need to log in before you can comment on or make changes to this bug.

Description Matej Starc 2022-01-19 17:28:26 UTC

Created attachment 145647 [details]
how to reproduce video

SUMMARY
A binding loop said to be in templates/private/ScrollView.qml line 133, property visible causes a binding loop.

STEPS TO REPRODUCE
1. Copy the code I provided.
2. Use qmlscene to execute it.
3. Resize from top right to bottom left as fast as possible (multiple if needed) as shown in the attached video. 

QML CODE
https://pastebin.com/S0MeFAme

OBSERVED RESULT
A crash/freeze caused by a binding loop.
https://pastebin.com/G9rXdEx4

SOFTWARE/OS VERSIONS
Linux/KDE Plasma: 
(available in About System)
Operating System: Arch Linux
KDE Plasma Version: 5.23.5
KDE Frameworks Version: 5.90.0
Qt Version: 5.15.2

Comment 1 Nate Graham 2022-02-04 16:38:38 UTC

*** Bug 449583 has been marked as a duplicate of this bug. ***

Comment 2 Nate Graham 2022-02-04 16:54:16 UTC

*** Bug 449581 has been marked as a duplicate of this bug. ***

Comment 3 Aleix Pol 2022-02-05 02:11:46 UTC

As observed by the reported, this is related to the resizing triggered by adding the scrollbar.

It happens where we put the window in that size that with the scrollbar there's  N columns and without N-1 or so it's seemed to me debugging 449583.

A solution would be to never take width to calculate columns when it can affect the resulting width (and same goes with height and rows).

Comment 4 Matej Starc 2022-02-07 10:20:22 UTC

(In reply to Aleix Pol from comment #3)
> As observed by the reported, this is related to the resizing triggered by
> adding the scrollbar.
> 
> It happens where we put the window in that size that with the scrollbar
> there's  N columns and without N-1 or so it's seemed to me debugging 449583.
> 
> A solution would be to never take width to calculate columns when it can
> affect the resulting width (and same goes with height and rows).

So it resizes infinitely(In reply to Aleix Pol from comment #3)
> As observed by the reported, this is related to the resizing triggered by
> adding the scrollbar.
> 
> It happens where we put the window in that size that with the scrollbar
> there's  N columns and without N-1 or so it's seemed to me debugging 449583.
> 
> A solution would be to never take width to calculate columns when it can
> affect the resulting width (and same goes with height and rows).

Sorry I had a hard time understanding the issue here..
So it tries to add a scrollbar (changes width and calculates how many rows it needs), but after that the scrollbar is not needed anymore, removes it (calculates how many rows it needs) and goes back to the first step ?

Comment 5 Aleix Pol 2022-02-14 15:35:15 UTC

Yes, that's my understanding of the problem, at least.

Comment 6 Nate Graham 2022-02-16 00:42:13 UTC

*** Bug 401533 has been marked as a duplicate of this bug. ***

Comment 7 Nate Graham 2022-03-22 01:45:36 UTC

*** Bug 451007 has been marked as a duplicate of this bug. ***

Comment 8 Nate Graham 2022-03-25 15:31:58 UTC

*** Bug 447080 has been marked as a duplicate of this bug. ***

Comment 9 Nate Graham 2022-03-25 15:32:02 UTC

*** Bug 446934 has been marked as a duplicate of this bug. ***

Comment 10 Nate Graham 2022-03-31 19:02:43 UTC

*** Bug 447958 has been marked as a duplicate of this bug. ***

Comment 11 Rajeesh K V 2022-04-01 05:05:59 UTC

A little bump here; to inform that this is currently (only) release blocker bug for Fedora 36. See https://bugzilla.redhat.com/show_bug.cgi?id=2057563

Comment 12 Nicolas Fella 2022-04-06 14:49:31 UTC

*** Bug 452326 has been marked as a duplicate of this bug. ***

Comment 13 Aleix Pol 2022-05-13 14:31:10 UTC

I've been looking into this issue and I was unable to reproduce it with the test case you suggested. Can you please confirm that it's still present?

Comment 14 Matej Starc 2022-05-14 21:44:04 UTC

(In reply to Aleix Pol from comment #13)
> I've been looking into this issue and I was unable to reproduce it with the
> test case you suggested. Can you please confirm that it's still present?
The fastest way I did it was to have 3 cards in a single row and the resize the window by moving the top up and down slowly scrollbar has to change visibility.

https://pastebin.com/pjU3KuYz

Comment 15 Aleix Pol 2022-05-16 10:53:09 UTC

This page is no longer available. It has either expired, been removed by its creator, or removed by one of the Pastebin staff.

:(

Comment 16 Matej Starc 2022-05-16 17:55:40 UTC

(In reply to Aleix Pol from comment #15)
> This page is no longer available. It has either expired, been removed by its
> creator, or removed by one of the Pastebin staff.
> 
> :(
This shouldn't expire or be removed until I delete it.
https://cloud.aljaxus.eu/index.php/s/aD7scnSQ58HcTNs

Comment 17 Matej Starc 2022-05-16 18:04:44 UTC

(In reply to Matej Starc from comment #16)
> (In reply to Aleix Pol from comment #15)
> > This page is no longer available. It has either expired, been removed by its
> > creator, or removed by one of the Pastebin staff.
> > 
> > :(
> This shouldn't expire or be removed until I delete it.
> https://cloud.aljaxus.eu/index.php/s/aD7scnSQ58HcTNs

This video makes the current cardsGridView even more concerning.

From what i've noticed ScrollBar doesn't disappear (around 25 sec), most of the app or whole goes black (around 43 sec and 48 sec) and finally activates the binding loop at 54 seconds.

https://cloud.aljaxus.eu/index.php/s/tP9rYwXXitzazNY

Comment 18 Bug Janitor Service 2022-05-20 12:27:38 UTC

A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kirigami/-/merge_requests/548

Comment 19 Matej Starc 2022-05-20 17:05:36 UTC

(In reply to Bug Janitor Service from comment #18)
> A possibly relevant merge request was started @
> https://invent.kde.org/frameworks/kirigami/-/merge_requests/548

Thanks, I will check it out.

Comment 20 Matej Starc 2022-05-20 17:29:24 UTC

(In reply to Bug Janitor Service from comment #18)
> A possibly relevant merge request was started @
> https://invent.kde.org/frameworks/kirigami/-/merge_requests/548

For some reason, I can't activate the binding loop anymore when using current arch stable.

Comment 21 Marco Martin 2022-06-03 08:52:41 UTC

Git commit d03c06bf7e047a822db66e8227a9a6bc060b2c67 by Marco Martin.
Committed on 03/06/2022 at 08:52.
Pushed by mart into branch 'master'.

Delete and port away from internal ScrollView

Refactor ScrollablePage to port it out of our internal implementation of ScrollView to upstream QQC2.ScrollView

port the other users as well, most notably OverlaySheet and Dialog.

Testing on various applications don't seem to produce any noticeable differences

M  +0    -2    src/CMakeLists.txt
M  +2    -1    src/controls/ContextDrawer.qml
M  +26   -28   src/controls/Dialog.qml
M  +161  -48   src/controls/ScrollablePage.qml
D  +0    -207  src/controls/private/RefreshableScrollView.qml
M  +0    -17   src/controls/templates/OverlayDrawer.qml
M  +17   -13   src/controls/templates/OverlaySheet.qml
D  +0    -161  src/controls/templates/private/ScrollView.qml

https://invent.kde.org/frameworks/kirigami/commit/d03c06bf7e047a822db66e8227a9a6bc060b2c67

Comment 22 Marco Martin 2022-07-14 12:11:55 UTC

Git commit c28dff79721d3805edcd413ef233216b3b3b0104 by Marco Martin.
Committed on 14/07/2022 at 12:11.
Pushed by mart into branch 'master'.

second attempt of Delete and port away from internal ScrollView

This reintroduces the refactor of ScrollablePage which was reverted due to last minute regressions.

Refactor ScrollablePage to port it out of our internal implementation of ScrollView to upstream QQC2.ScrollView

port the other users as well, most notably OverlaySheet and Dialog.

Testing on various applications don't seem to produce any noticeable differences

M  +0    -2    src/CMakeLists.txt
M  +2    -1    src/controls/ContextDrawer.qml
M  +26   -28   src/controls/Dialog.qml
M  +207  -47   src/controls/ScrollablePage.qml
D  +0    -207  src/controls/private/RefreshableScrollView.qml
M  +0    -17   src/controls/templates/OverlayDrawer.qml
M  +26   -14   src/controls/templates/OverlaySheet.qml
D  +0    -161  src/controls/templates/private/ScrollView.qml

https://invent.kde.org/frameworks/kirigami/commit/c28dff79721d3805edcd413ef233216b3b3b0104