Caret Handling when Closing Menus
Pages: 1 2
Steve Fryatt (216) 2095 posts |
This puzzled me at the time, but I put it down to an issue with WinEd that had been around for a while and not been noticed… Except that now I’ve just found exactly the same behaviour in CashBook, and I’m pretty confident that it didn’t always do it — because I tested the “save on close” functionality when I implemented it. Has something changed in RISC OS with the way that the caret is handled when menu trees close? Perhaps as part of the work on writable icons? ETA… Thinking about it, it shouldn’t really be up to the app to look out for this. If the caret came from a window owned by another application, and that is closed while the menu tree is open, no amount of careful ordering of the window closing action will avoid the crash. It’s unlikely, I agree, as it would have to close without user intervention, but it feels a bit fragile nonetheless. |
Steve Pampling (1551) 8125 posts |
My memory says that someone else raised an issue with caret placement etc wrt the writable icon changes, or was it the clipboard stuff? |
Andy S (2979) 504 posts |
People were talking about similar crashes in this thread but it doesn’t look like that got resolved there. |
Andy Vawer (5817) 28 posts |
I’ve submitted a merge request and would be grateful if anyone affected can give it a try too. If you are compiling from sources, the only difference is the addition of CLRV in line 6347 of s/Wimp05 in the Wimp source. |
Andy S (2979) 504 posts |
I don’t know yet if it impacts the Paint rename killing Filer issue as I haven’t been able to replicate that here yet. Thanks for the fix, Andy. I’ve not been able to replicate that either but hopefully it will resolve the issues Rick Murray’s been having. |
Matthew Phillips (473) 709 posts |
A RiscOSM user has been having problems with software crashing and we have recently been able to reproduce this on our RPi3. From the debugging we have added to each application it seems that the crashing is happening in the Wimp rather than either application, and it may be significant that the crash happens when a menu containing the caret is closed. The sequence of moves needed to trigger the crash is:
(Application A is a particular application written by the user who is having the problems. It is in BASIC and it doesn’t look like it is doing anything weird.) If you do “Describe” and then try to Quit you get a whole series of applications, named and unnamed, falling over. If instead of “Describe” you do “Quit” then Application A disappears, and RiscOSM redraws the map at the new scale, but interestingly the Scale menu remains on screen (despite our having left-clicked to select the new scale) and if you wander the mouse over it, to highlight another menu option, then you get more “Application may have gone wrong.” type messages. By adding logging to each application we can see that Application A gets a mouse click (whereupon it claims the caret) and then a bit later receives reason code 11 to say that it has lost the caret. This must be when RiscOSM’s menu has opened. There are no further log lines, so Application A never gets control back from its Wimp_PollIdle. But as the crash appears to happen in Application A, the Wimp must have paged the task in and been about to return from Wimp_Poll — or at least, I would guess that is what is happening. I would be expecting a GainCaret at that point. In RiscOSM the application gets the Menu Selection event, and does one or two minor things in response to that. It changes its poll mask to enable null events, because the map data loading is time-consuming so it does it in chunks while polling. But the crash happens before RiscOSM’s call to Wimp_Poll returns. We have reproduced the error the RPi3 running RISC OS 5.28 and also 5.29 (19 Sep 2021) which includes Window Manager 5.83 (13 Sep 2021). From my reading of GitLab that would include Andy Vawer’s latest fix . We have not managed to reproduce the error on 5.16 (Iyonix), 5.23 (ARMX6) or on 5.25 (RPi3, 11 May 18). I don’t have any other versions handy. Any ideas? |
Matthew Phillips (473) 709 posts |
The problem is reproducible if Application A is replaced with Edit, Messenger Pro’s EmailEdit or Ovation Pro. If any of those applications has the input focus, then after opening and selecting from RiscOSM’s scale menu, we get a crash in the application that had the input focus previously. We’ve not yet succeeded in creating a minimal application to replace RiscOSM’s part in the crash, so the possibility must remain that there is a fault in RiscOSM rather than the Wimp. One very frustrating thing is that if we run WimpMon to track the Wimp messages being exchanged during the process, the problem goes away. |
Matthew Phillips (473) 709 posts |
We have created a very cut-down RiscOSM (renamed TestMenu) which you can download to see the problem. We’d be very interested to know which versions of RISC OS experience a crash. The application does very little indeed: it places an icon on the iconbar. Clicking with any mouse button will open a menu which includes a writable item. Only the Quit option does anything. You can download it from https://sinenomine.co.uk/software/testmenu.zip To reproduce the problem:
On RISC OS 5.28 onwards you will get an error such as “Internal error, no stack for trap handler: Internal error: abort on data transfer at &FC16AE9C, pc = 001375D8: registers at 2036C234.” After clicking Cancel in the error box you then will find Edit will quit, and possibly a few other bits of the operating system will fall over too. The !RunImage of the application is quite big – about 1.4MB. This is because it still has all of RiscOSM’s code linked in at compile time, but the vast majority of it is not being called. We found that if we only included the few parts of the application still in use (mainly the Wimp library, event and menu handling) the crash did not occur. We can only reproduce the crash with a big !RunImage. We have tried increasing the Wimp slot massively, but that made no difference. In any case, on recent OS versions the Wimp slot size is worked out based on information in the Absolute. All that the application is doing is:
This very much looks like a bug in the Wimp. We have not stripped down the event handling library and other code to the minimum yet, but it’s used in all our applications and we have not had any problems with them. It seems to be a combination of a large !RunImage (or large Wimp slot) together with the writable item on a menu taking the input focus from another application. |
Stuart Swales (8827) 1323 posts |
That kills all the desktop tasks on my ARMX6, 5.29 (OS18), right back to the supervisor prompt! Tried it with PipeDream as the ‘editor’ too; it borks as with Edit. [Edit #1: it seems to go wrong when closing the menu and handing the caret back – if you right click one of the dummy menu entries it is OK.] [Edit #2: doesn’t seem to go wrong with SrcEdit and Edit loaded with large files (>3MB slots) and choosing options from the icon menu] |
David Pitt (3386) 1248 posts |
A quick look on the Titanium shows OS5.28 bad but OS5.27 23Apr19 good, so that is somewhere between Wimp 5.62 and 5.80. I don’t have any intermediate Titanium ROMs to hand. |
Dave Higton (1515) 3477 posts |
Are you running ZeroPain? The AODT might be a zero page access, which ZeroPain would catch and perhaps prevent a crash. I don’t pretend ZeroPain to be a cure, but it might be a useful diagnostic. |
Matthew Phillips (473) 709 posts |
I’ve checked OS_PlatformFeatures 0 bit 20 and it’s running with high processor vectors. I installed ZeroPain in !Boot.Choices.Book.PreDesk and reset the machine. Confirmed with “*help modules” that ZeroPain was present. Triggered the crash, but no ZeroPain reports on the boot disc. I’m using the Rick-modified ZeroPain available from http://www.heyrick.co.uk/random/zeropain_noexpire.zip @Stuart: Very interesting discovery that there is no crash if Edit has a very large document and therefore Wimp slot. I reckon what must be happening is that after responding to the menu click RiscOSM calls Wimp_Poll. Perhaps then the Wimp pages out RiscOSM, but then, when handling the caret relocation, tries to read from the icon buffer (which is in RiscOSM’s Wimp slot). If the new task which has been paged in does not have a very big slot, we get the crash, but if the next task has a large slot, the read occurs without error. (I do not know whether this is plausible.) |
Matthew Phillips (473) 709 posts |
I’ve reported it as a bug where I have added the information that if RiscOSM claims the caret (e.g. in its main window) just before opening the menu that contains the writable item, we do not get a crash. |
Rick Murray (539) 13747 posts |
I had raised this possibility earlier as there’s no logical way for Paint’s menu shenanigans to take out the Filer (and some other stuff). |
Rick Murray (539) 13747 posts |
To follow up, something similar and yes, it looks like weird task switching going on. |
Martin Avison (27) 1479 posts |
That has not been the case for some time – I have been runing v0.10 (30 May 2020) since then, on RO5.28 and recent 5.29. |
Matthew Phillips (473) 709 posts |
Armed with the insight from Stuart above that the size of the Wimp slot is the crucial thing, we’ve created a BASIC test application that demonstrates the problem. Follow the same steps as above but now you can verify that the application is not doing anything wrong, and it must be a Wimp bug. |
Stuart Painting (5389) 707 posts |
Testing with RPCEmu shows that the 17 Nov 2019 ROM (Wimp 5.64) is OK, but the 04 Jan 2020 ROM (Wimp 5.66) fails as described above. I don’t have any December 2019 ROMs so I can’t tell which of Wimp 5.65 or Wimp 5.66 first exhibited the problem. |
Matthew Phillips (473) 709 posts |
Sadly, the change from 5.64 to 5.66 includes the introduction of clipboard support which was over 8000 lines of assembly language. I think we’re going to need a Wimp expert to look at this one! |
Andy Vawer (5817) 28 posts |
Thanks for finding an easily repeatable way of crashing the Wimp – makes it easier to investigate! I’ve put in a merge request for a possible fix, so you may wish to give it a try. The issue is that when closing menus, set_caret_position is called to remove the caret from the menu tree. This tries to redraw the icon without a caret as the window is still defined and valid at this time. However, the Wimp internals have ditched the task handle of the menu owner by this time, so the redraw can’t page in the right task (Menu windows are marked internally as a task handle of -1 and the real task handle is stored elsewhere). The redraw then fails horribly trying to access memory from the wrong task. Everything then falls apart :( The fix is to ensure that the correct task is paged in. If it’s a menu then check the menu task handle. If this isn’t valid then the menu’s being deleted anyway so there’s no point in doing a redraw at all. Fingers crossed… |
David Pitt (3386) 1248 posts |
Looking good here on a Titanium ROM build, neither of the |
Matthew Phillips (473) 709 posts |
Quick work! I think it’s going to take me a bit of time getting my head round downloading the fix and compiling it up into something I can test as I’ve only once worked with the RISC OS source before. But thank you for working out the solution so quickly. |
David Pitt (3386) 1248 posts |
|
Matthew Phillips (473) 709 posts |
Thank you David, very helpful. |
David Pitt (3386) 1248 posts |
The ‘exploding menu’ fix has not yet made it into the beta ROMs. The current beta, 17 Oct 21, does fail as described above.
|
Pages: 1 2