Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(DiskBar): simplify and prevent invalid constraints #862

Merged
merged 5 commits into from
Dec 12, 2024

Conversation

davidmhewitt
Copy link
Member

@davidmhewitt davidmhewitt commented Dec 11, 2024

Fixes #848

Turns out the GTK constraint solver crashes (unpredictably? not always) when you give it invalid constraints, so I've made multiple improvements here to make the code clearer and remove most (all?) of the possibilities where we could be passing invalid constraints.

The main issue was clamping the percentages between 1% and 99%. Imagine a case where a disk has 3 partitions and the first two partitions use <1% of the disk and the 3rd partition uses >99%. We then clamp these percentages to [1, 1, 99], which according to my calculations adds up to 101%. This makes the GTK constraint solver sad.

Changes

  • Add a more complicated mock disk layout to the test data in PartitioningView.vala, this allows reproducing the crashing issue in Installer crashes when navigating to the custom installation view with a system storage with custom partition setup #848 prior to the other changes, and should help test for regressions in future.
  • Make it clear in variable names and parameter names when we're using sectors vs actual size in bytes
  • Calculate percentages consistently based on sector sizes, and don't mix sectors/bytes in calculations
  • Don't hardcode a 512 sector size
  • Don't clamp the percentages (the image widget inside the partition block widget reserves space, so we don't get 0 sized widgets even with really tiny percentages)

src/Widgets/DiskBar.vala Outdated Show resolved Hide resolved
Copy link
Member

@vjr vjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not able to repro myself but just reading the diff LGTM - just the one comment posted. Presume you were able to repro and verify the fix.

@Hibryda
Copy link

Hibryda commented Dec 11, 2024

May be related to #861 as well

@danirabbit
Copy link
Member

danirabbit commented Dec 11, 2024

Hm I'm actually seeing a crash here with output:

(io.elementary.installer:7860): Gtk-CRITICAL **: 13:58:27.859: INTERNAL: ratio == DBL_MAX in dual_optimize

(io.elementary.installer:7860): Gtk-CRITICAL **: 13:58:27.859: INTERNAL: invalid entry variable during pivot

(io.elementary.installer:7860): Gtk-CRITICAL **: 13:58:27.859: gtk_constraint_expression_change_subject: assertion 'new_subject != NULL' failed

backtrace:

#0  0x00007ffff72c2c2f in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#1  0x00007ffff72c2d2d in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#2  0x00007ffff72ca1e1 in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#3  0x00007ffff7110a5e in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#4  0x00007ffff7187a90 in gtk_layout_manager_measure () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#5  0x00007ffff721b86a in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#6  0x00007ffff70eac33 in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#7  0x00007ffff7187a90 in gtk_layout_manager_measure () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#8  0x00007ffff721b86a in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#9  0x00007ffff70eac33 in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#10 0x00007ffff7187a90 in gtk_layout_manager_measure () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#11 0x00007ffff721b86a in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#12 0x00007ffff721b74e in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#13 0x00007ffff71fa451 in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#14 0x00007ffff721b74e in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#15 0x00007ffff722949f in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#16 0x00007ffff721b74e in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#17 0x00007ffff70eac33 in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#18 0x00007ffff7187a90 in gtk_layout_manager_measure () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#19 0x00007ffff721b86a in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#20 0x00007ffff6a4ef1c in ??? () at /lib/x86_64-linux-gnu/libadwaita-1.so.0
#21 0x00007ffff7187a90 in gtk_layout_manager_measure () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#22 0x00007ffff721b86a in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#23 0x00007ffff70eae8f in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#24 0x00007ffff7187a90 in gtk_layout_manager_measure () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#25 0x00007ffff721b86a in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#26 0x00007ffff70eac33 in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#27 0x00007ffff7187a90 in gtk_layout_manager_measure () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#28 0x00007ffff721b86a in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#29 0x00007ffff70e1e67 in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#30 0x00007ffff7187a90 in gtk_layout_manager_measure () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#31 0x00007ffff721b86a in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#32 0x00007ffff6a6dfff in ??? () at /lib/x86_64-linux-gnu/libadwaita-1.so.0
#33 0x00007ffff721b74e in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#34 0x00007ffff729cd0f in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#35 0x00007ffff70d57cb in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#36 0x00007ffff721b74e in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#37 0x00007ffff72a5bc9 in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#38 0x00007ffff7e9c2fa in g_closure_invoke () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#39 0x00007ffff7ecb90c in ??? () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#40 0x00007ffff7ebc591 in ??? () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#41 0x00007ffff7ebc7c1 in g_signal_emit_valist () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#42 0x00007ffff7ebc883 in g_signal_emit () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#43 0x00007ffff742d00c in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#44 0x00007ffff7490dd8 in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
--Type <RET> for more, q to quit, c to continue without paging--
#45 0x00007ffff7ebc6bd in ??? () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#46 0x00007ffff7ebc7c1 in g_signal_emit_valist () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#47 0x00007ffff7ebc883 in g_signal_emit () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#48 0x00007ffff747475b in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#49 0x00007ffff7474f2e in ??? () at /lib/x86_64-linux-gnu/libgtk-4.so.1
#50 0x00007ffff7d9c522 in ??? () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#51 0x00007ffff7d9b48e in ??? () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#52 0x00007ffff7dfa717 in ??? () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#53 0x00007ffff7d9aa53 in g_main_context_iteration () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#54 0x00007ffff6f1788d in g_application_run () at /lib/x86_64-linux-gnu/libgio-2.0.so.0
#55 0x0000555555565b70 in _vala_main (args=0x7fffffffd9a8, args_length1=1) at ../src/Application.vala:95
#56 0x0000555555565bd1 in main (argc=1, argv=0x7fffffffd9a8) at ../src/Application.vala:81

Running with --test works perfectly though!

@davidmhewitt
Copy link
Member Author

Yeah, so this is the same crash and backtrace as before.

I wonder if this is not necessarily related to invalid constraints but some race condition or binding issue with Vala. Fixing up the constraints definitely improved things for me, but it wasn't 100% reproducible to begin with.

If you're running the Daemon somewhere where you've got access to d-feet, could you run the GetDisks method on the installer system bus with argument "True" and drop the output here? I'll look at what constraints we're generating for your disk layout and see if there's anything I missed, but I probably need to do some more digging here.

@danirabbit
Copy link
Member

(([('Samsung SSD 970 EVO 500GB', '/dev/nvme0n1', uint64 976773168, uint64 512, false, false, [('/dev/nvme0n1p1', 7, uint64 4096, uint64 542966, (byte 0x01, uint64 30312), ''), ('/dev/nvme0n1p2', 11, 542968, 976769070, (0x00, 0), 'data')])], [('LVM data', '/dev/mapper/data', uint64 976226102, uint64 512, false, false, [('/dev/dm-0', 4, uint64 0, uint64 968220672, (byte 0x01, uint64 534569520), ''), ('/dev/dm-1', 9, 968220673, 976216065, (0x00, 0), '')])]),)

@davidmhewitt
Copy link
Member Author

@danirabbit Thank you! I was able to reproduce the crash again with your disk layout. Should now be resolved for all cases.

Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect! Thank you for fixing this!

@danirabbit danirabbit merged commit e6fa262 into main Dec 12, 2024
5 checks passed
@danirabbit danirabbit deleted the david_diskbar_fixups branch December 12, 2024 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants