Caret Handling when Closing Menus

This appears to be a side effect of the Wimp’s automatic handling of writable icons in transient dialogue boxes. Open a transient window with a writable icon (or just a writable menu field), and the Wimp will move the caret into it. When the window closes, the Wimp will put the caret back where it came from. If the window no longer exists, such as if the application has just closed and deleted it as a result of the menu action, then Wimp_CreateMenu throws an “Illegal Window Handle” error before it gets around to destroying the menu tree. DeskLib calls the X form of the SWI, and doesn’t pass the error back to the application.

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.

Perhaps as part of the work on writable icons?

My memory says that someone else raised an issue with caret placement etc wrt the writable icon changes, or was it the clipboard stuff?

People were talking about similar crashes in this thread but it doesn’t look like that got resolved there.

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.
This stops the menu destruction failing when there’s an invalid return destination for the caret. It fixes a potentially catastrophic cascade of Illegal Window handle errors. 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.

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.

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:

1. Click on window of application A to give it input focus
2. Click on a menu icon in RiscOSM to select a new map scale. This opens a menu where the last item is writable (allowing you to type your own choice of scale).
3. Choose one of the ordinary scales from the menu.
4. There is then a “Message from application A” saying “Application A may have gone wrong. Click Continue to try to resume of Quit to stop application A”
5. “Describe” gives “Internal error: abort on data transfer at &FC16AE9C – Internal Error Code 1491”

(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?

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.

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:

1. Run TestMenu
2. Run Edit
3. Open an editor window and make sure it has the input focus.
4. Click on TestMenu’s icon, and choose any item from the menu.

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:

1. Set signal handlers
2. Wimp_Initialise
3. Sets various global variables like the application name
4. Creates the iconbar menu data structure
5. Adds its icon to the iconbar
6. Registers a handler in the event handling system for clicks on the iconbar icon
7. Loops polling the Wimp
8. If a click is received, it opens the menu. If the user clicks on Quit, the application shuts down.

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.

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]

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.

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.

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
rather than the one from the Bonus Binaries download as I understood from something I read that the official one expired. Perhaps I am wrong about that.

@Stuart:
It’s interesting that nothing goes wrong if you adjust-click on the menu. Obviously our code is reopening the menu as part of the response to the menu click event, as that’s what you’re supposed to do. I guess that means the Wimp does not have to relocate the caret.

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.)

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.

(I do not know whether this is plausible.)

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).
That happening would only seem to make sense if the Wimp was doing something very strange with task switching, and not putting the right things back in the right place?

To follow up, something similar and yes, it looks like weird task switching going on.

https://www.riscosopen.org/tracker/tickets/519

ZeroPain … I read that the official one expired.

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.

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.

we’ve created a BASIC test application that demonstrates the problem.

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.

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!

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…

Fingers crossed…

Looking good here on a Titanium ROM build, neither of the TestMenu demos crash now.

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.

Looking good here on a Titanium ROM build, neither of the TestMenu demos crash now.

A Pi ROM with the fix is here.

Thank you David, very helpful.

The ‘exploding menu’ fix has not yet made it into the beta ROMs. The current beta, 17 Oct 21, does fail as described above.

A Pi ROM with the fix is here.