Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#128 closed bug (fixed)

StackCheck (rework required)

Reported by: Mikhail Malyshev Owned by: Thore Böckelmann
Priority: normal Milestone: 3.9-2015R1
Component: Application.mui Version: 3.9-nightly build
Severity: minor Keywords:
Cc: OS Platform: AmigaOS3
Blocked By: Blocking:
Release Notes:

Description

Summary

Causes false alerts, crashes on program termination.

Steps to reproduce

eg. SysSpeed (sets up own stack 16K, icon has no effect) (hits/crash if cancel)

mFTP (set 32K in icon → get warning since part of stack is in use on start ~30K avail!)

Change History (12)

comment:1 Changed 5 years ago by Mikhail Malyshev

Proposal: Make stack check user definable value

comment:2 Changed 5 years ago by Thore Böckelmann

Component: undefinedApplication.mui
Milestone: MUI 3.9-2014R4
Owner: set to Thore Böckelmann
Priority: undecidednormal
Status: newassigned

Making the requires size user defineable does not make any sense. If the user can adjust it smart people will lower that user size until the requester goes away, but this will not cure the possible stack overflows. It will just hide them as they were hidden before.

And if an application crashes after clicking on "Exit immediately" then this happens, because the programmer was too lazy to check for a valid application object. The application would have crashed as well if creating the application object failed for any other reason.

However, what can be done is to check the process' stack size as well. So far only the task's stack size was checked and these two values might differ if certain patches are running.

comment:3 Changed 5 years ago by Thore Böckelmann

Resolution: fixed
Status: assignedclosed

In 4281:

  • Application.c: incorporate the process' stack size in the stack check as well. This closes #128.

comment:4 Changed 5 years ago by Mikhail Malyshev

I hope you check for allocated stack, and not for free stack.
My last test with mFTP showed that the warning appears with 32K in icon setting,
but when apps starts the stack is 32K with 2K already used, and thus a warning!
Setting stack to 34K removes the warning. So imho the logic is a bit wrong, since otherwise we will be wasting 32K most of the time.

comment:5 Changed 5 years ago by Mikhail Malyshev

Resolution: fixed
Status: closedreopened

Stack warning is missing translation in 3.9 builds, text is translated in 4.0 files only.

Vapor apps fail the stack check… eg. AmFTP also does not like aborting low stack and gives enforcers on exit.

comment:6 Changed 5 years ago by Jens Maus

Resolution: fixed
Status: reopenedclosed

Sorry Michael, but Thore already told you that there isn't any way to stop application from crashing after you acknowledge the stack warning requester. This is simply due to bugs in these applications and have to be fixed there instead. So simply increase the stack size of these applications and the problem will vanish.

comment:7 Changed 5 years ago by Mikhail Malyshev

Resolution: fixed
Status: closedreopened

The bug report relates more to missing locale, so the bug can be closed when the locale is fixed.

The second comment is just a note.

Another idea comes to mind, if the program started with a too low stack,
why can't we issue a warning, and then auto increase it in stackattack fashion?
Would solve the problem of termination, since it will not be required.

comment:8 Changed 5 years ago by Jens Maus

Resolution: fixed
Status: reopenedclosed

The bug report is about the Stack warning mechanism and not about the missing locale translation. Review the description of the ticket. If a locale translation is missing issue a new ticket and don't hijack this ticket for it. The same applies for your ideas. This is no discussion fora but a simple ticket system. Open a new ticket for every new thing you want to raise. You have been told a hundred times already about that. And no, it doesn't make sense to increase the stack like StackAttack. MUI is graphical user interface framework and not a system tool trying to deal with stack. If the warning comes up → go to the application and increase the requested stack size there (in the icon, etc.).

comment:9 Changed 5 years ago by Thore Böckelmann

A final comment from my side. There won't be a translated warning for MUI 3.9. The reason is that MUI 3.9 has separated muimaster.library and muilocale.library. The latter must be opened each time a translation is required and as with any other library this might fail for any reason. This possible failure must be checked and handled and finally we end up with duplicate code and text for a warning requester which (in the best case) should never become visible at all.

For MUI4 the situation is a bit different, because there the catalog handling is built into muimaster.library and getting the translated text cannot fail.

Finally, acting the same way like StackAttack is no option and not possible. StackAttack influences the creation of a process and will increase the requested amount of stack before the process is actually created. Once the process is running StackAttack will not touch it anymore. Doing the increase later (if that is possible without breaking any stuff that has been created on the stack already) will not solve the initial problem, because at the time MUI is checking the stack size the stack might have been overrun already and MUI's warning comes too late. And the fact that certain applications will crash if creating the Application object fails is absolutely unrelated to a too low stack size, but only to the laziness and cleverness of too many developers who think that certain things will always succeed. This is plain wrong and would have crashed as well before I added the check.

comment:10 Changed 5 years ago by Mikhail Malyshev

That's a very comprehensive comment. Thank you.

comment:11 Changed 5 years ago by Thore Böckelmann

Milestone: MUI 3.9-2014R4MUI 3.9-2015R1

Milestone renamed

comment:12 Changed 5 years ago by Thore Böckelmann

Milestone: MUI 3.9-2015R13.9-2015R1

Milestone renamed

Note: See TracTickets for help on using tickets.