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

Planner / profile hang #2212

Closed
1 of 4 tasks
bstoeger opened this issue Aug 5, 2019 · 5 comments · Fixed by #2217
Closed
1 of 4 tasks

Planner / profile hang #2212

bstoeger opened this issue Aug 5, 2019 · 5 comments · Fixed by #2217
Assignees

Comments

@bstoeger
Copy link
Collaborator

bstoeger commented Aug 5, 2019

screenshot

Describe the issue:

  • Bug
  • Change request
  • New feature request
  • Discussion request

Issue long description:

I can easily make the planner hang on save:
Create a crazy profile such as in the attached screen shot. Note the crazy pressure values. When saving the plan the application hangs. This happens on an empty dive log. I'm not sure if this is relevant.

Operating system:

Kubuntu 19.4

Subsurface version:

Current master.

Steps to reproduce:

  1. Open empty log
  2. Create crazy plan as per attached screenshot
  3. Save plan

Current behavior:

Hang

Expected behavior:

No hang

Additional information:

Backtrace:
#0 0x00007ffff2f39f8d in ?? () from /lib/x86_64-linux-gnu/libQt5Core.so.5
#1 0x00007ffff2f3cabe in QObject::connect(QObject const*, char const*, QObject const*, char const*, Qt::ConnectionType) ()
from /lib/x86_64-linux-gnu/libQt5Core.so.5
#2 0x00005555557c5e00 in DiveTextItem::setText (this=this@entry=0x55557bbffba0, t=...)
at /home/bs/src/subsurface/profile-widget/divetextitem.cpp:63
#3 0x00005555557b9019 in DiveCartesianAxis::updateTicks (this=0x5555567f2030, color=TIME_GRID)
at /home/bs/src/subsurface/profile-widget/divecartesianaxis.cpp:207
#4 0x00005555557b7857 in DiveCartesianAxis::animateChangeLine (this=0x5555567f2030, newLine=...)
at /home/bs/src/subsurface/profile-widget/divecartesianaxis.cpp:266
#5 0x00005555557a464a in ProfileWidget2::setProfileState (this=0x55555669c140) at /home/bs/src/subsurface/profile-widget/profilewidget2.cpp:1236
#6 0x0000555555691392 in MainWindow::showProfile (this=0x555555da6630) at /home/bs/src/subsurface/desktop-widgets/mainwindow.cpp:744
#7 0x00005555556913e9 in MainWindow::planCreated (this=0x555555da6630) at /home/bs/src/subsurface/desktop-widgets/mainwindow.cpp:830
#8 0x00005555556dfa88 in MainWindow::qt_static_metacall (_o=0x555555da6630, _c=, _id=, _a=)
at /home/bs/src/subsurface/build/desktop-widgets/subsurface_interface_autogen/EWIEGA46WW/moc_mainwindow.cpp:380
#9 0x00007ffff2f37426 in QMetaObject::activate(QObject*, int, int, void**) () from /lib/x86_64-linux-gnu/libQt5Core.so.5
#10 0x00005555557e614b in DivePlannerPointsModel::createPlan (this=0x555555b2cf00 DivePlannerPointsModel::instance()::self,
replanCopy=) at /home/bs/src/subsurface/qt-models/diveplannermodel.cpp:1143
#11 0x00005555557ff577 in DivePlannerPointsModel::qt_static_metacall (_o=0x555555b2cf00 DivePlannerPointsModel::instance()::self,
_c=, _id=, _a=0x7fffffffcc70)
at /home/bs/src/subsurface/build/qt-models/subsurface_models_desktop_autogen/EWIEGA46WW/moc_diveplannermodel.cpp:321
#12 0x00007ffff2f37426 in QMetaObject::activate(QObject*, int, int, void**) () from /lib/x86_64-linux-gnu/libQt5Core.so.5
#13 0x00007ffff46717e0 in ?? () from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#14 0x00007ffff2f37426 in QMetaObject::activate(QObject*, int, int, void**) () from /lib/x86_64-linux-gnu/libQt5Core.so.5
#15 0x00007ffff45d2032 in QAbstractButton::clicked(bool) () from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#16 0x00007ffff45d224a in ?? () from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#17 0x00007ffff45d360f in ?? () from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#18 0x00007ffff45d37e5 in QAbstractButton::mouseReleaseEvent(QMouseEvent*) () from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#19 0x00007ffff4528b58 in QWidget::event(QEvent*) () from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#20 0x00007ffff44e9551 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#21 0x00007ffff44f0b77 in QApplication::notify(QObject*, QEvent*) () from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#22 0x00007ffff2f0d8e9 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /lib/x86_64-linux-gnu/libQt5Core.so.5
#23 0x00007ffff44efebf in QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer&, bool, bool) () from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#24 0x00007ffff454357b in ?? () from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#25 0x00007ffff454653f in ?? () from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#26 0x00007ffff44e9551 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#27 0x00007ffff44f0930 in QApplication::notify(QObject*, QEvent*) () from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#28 0x00007ffff2f0d8e9 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /lib/x86_64-linux-gnu/libQt5Core.so.5
#29 0x00007ffff3a48c6c in QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::MouseEvent*) ()
from /lib/x86_64-linux-gnu/libQt5Gui.so.5
#30 0x00007ffff3a4a075 in QGuiApplicationPrivate::processWindowSystemEvent(QWindowSystemInterfacePrivate::WindowSystemEvent*) ()
from /lib/x86_64-linux-gnu/libQt5Gui.so.5
#31 0x00007ffff3a2405b in QWindowSystemInterface::sendWindowSystemEvents(QFlagsQEventLoop::ProcessEventsFlag) ()
from /lib/x86_64-linux-gnu/libQt5Gui.so.5
#32 0x00007fffec32167a in ?? () from /lib/x86_64-linux-gnu/libQt5XcbQpa.so.5
#33 0x00007ffff0f889ee in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#34 0x00007ffff0f88c88 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#35 0x00007ffff0f88d1c in g_main_context_iteration () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#36 0x00007ffff2f61047 in QEventDispatcherGlib::processEvents(QFlagsQEventLoop::ProcessEventsFlag) ()
from /lib/x86_64-linux-gnu/libQt5Core.so.5
#37 0x00007ffff2f0c5bb in QEventLoop::exec(QFlagsQEventLoop::ProcessEventsFlag) () from /lib/x86_64-linux-gnu/libQt5Core.so.5
#38 0x00007ffff2f145e2 in QCoreApplication::exec() () from /lib/x86_64-linux-gnu/libQt5Core.so.5
--Type for more, q to quit, c to continue without paging--
#39 0x00005555556847e0 in main (argc=, argv=) at /home/bs/src/subsurface/subsurface-desktop-main.cpp:116

Mentions:

@atdotde

@atdotde
Copy link
Collaborator

atdotde commented Aug 6, 2019

I cannot reproduce this. Unfortunately, you screenshot does not show your "crazy cylinder values".

@atdotde
Copy link
Collaborator

atdotde commented Aug 6, 2019

OK, now, I get it: Your cylinder is so small that the end pressure under-runs the minimal value of cylinder.end.mbar which is a pressure_t which is an int32, so it goes to about -2000bar before under-running.

I see two possible approaches to a fix:

a) Don't let the planner go to negative pressures. When the cylinder is empty, it's empty. This amounts to adding

if (cyl->end.mbar < 0) cyl->end.mbar =0;

in the end of update_cylinder_pressure in planner.c.

Disadvantage: You only get your gas use in volume units not a reasonable negative pressure (which would inform you how many more cylinders you would need. A somewhat more pragmatic approach would be to allow some negative values but not crazy negative, for example limit to minus working pressure.

b) give more bits to the end pressure. I tried making pressure_t an int64 but that was not enough. I guess one would also need to look at the code that does the pressure interpolation to use long all along.

Any opinions?

@torvalds
Copy link
Collaborator

torvalds commented Aug 7, 2019

I wrote a longer email on the list. No, pressure_t isn't the problem.

I suspect some intermediate calculation multiplies depth (in mm) and pressure (in mbar) together (or possibly cylinder volume in mm), and doesn't reduce ranges, so when while both values comfortably fit in their respective types, the intermediate value from the multiplication of two 32-bit values will overflow. And while it gets converted back to some pressure later (possibly by just fixing up the range by a fixed unit divisior), by that time the overflow has long since happened already and the end result is random.

@torvalds
Copy link
Collaborator

torvalds commented Aug 7, 2019

Some of the craziness is mitigated by

#2214

but that's really only band-aid and makes sure that when the planner starts using crazy values, the gas compressibility code doesn't then push the crazy values even further.

The real fix is to make the planner not use negative gas pressures. So I'm not closing this bug report.

@atdotde
Copy link
Collaborator

atdotde commented Aug 8, 2019

I believe #2214 is actually a fix for the problem. As said before, negative pressures make sense as they inform the user how much gas is actually missing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants