herzbube / littlego

Little Go. An iOS application that lets you play the game of Go on the iPhone or iPad.
https://littlego.herzbube.ch/
Apache License 2.0
138 stars 53 forks source link

Crash when board setup changes but board position is not 0 #366

Closed herzbube closed 2 years ago

herzbube commented 2 years ago

The application requires that board setup is made before any actual moves occur. The UI is therefore built to prevent board setup actions by the user once the board display a board position beyond 0. As a final safeguard against violations of the rule, the class HandleBoardSetupInteractionCommand contains a check that, if it fails, raises an exception that causes the application to crash.

Since the release of version 1.6.0 of the app, a relatively high number of crashes (about 100 crashes from 17 different users in the last 90 days) have been reported that are due to this check. The issue needs to be investigated and fixed.

herzbube commented 2 years ago

The only source that triggers the execution of HandleBoardSetupInteractionCommand is the method handleTapFrom: in TapGestureController. The trigger occurs only if the property UiSettingsModel.uiAreaPlayMode has value UIAreaPlayModeBoardSetup. The intention is that the user can activate this mode only if the board position is currently 0, and that the app changes the mode to something else (usually UIAreaPlayModePlay) when the user navigates to a board position beyond 0.

No obvious code defects were found that allow the user to violate the mode condition. Some cases were found, however, where the user can circumvent the condition by making iOS recognize a tap gesture while another tap gesture is already in progress.

Case 1

Case 2

Case 3

Case 4

Note: Originally it was thought that these cases cannot be reproduced on a big iPhone (UITypePhone) because there tapping + holding the forward button is blocking other interactions. This was found not to be the case, i.e. the two cases can be reproduced on all devices.

herzbube commented 2 years ago

The cases described in the previous comment require the user to perform quite convoluted actions, so it seems unlikely that these are the causes for all the crashes that occur in the field. It seems more likely that other, simpler ways to provoke the crash exist. However, even if further causes are found, any conclusive solution for the issue requires that the described cases are also handled.

The root cause for the described cases is that iOS allows further gestures while the user is performing a tap+hold gesture on a toolbar button. I currently can't think of a way how to prevent that since the application is not in control of handling toolbar button tap gestures. While it might be possible to somehow hook into iOS' gesture handling on a low level, the approach feels too complicated. A much simpler solution is to remove the cause of the crash.

Decision: