Accidental file delete on mobile.

[Dirk Hohndel]

While cursing into my beard about a completely unrelated issue it suddenly occurred to me that I was almost certainly trying to solve the wrong problem.
It's not the deleteAction that has the signal handler that's still running. Because that action isn't deleted.
What I am now speculating that is happening is slightly more convoluted (I absolutely shudder trying to make sense of all this... what a mess).
So we add this action to the context menu. We do this by adding it to an array of actions that is part of the contextDrawer structure.
From this array of actions the code now builds a repeater that has delegates that implement the different entries in the contextDrawer.
My theory now is that while the manager.deleteDive() is running the dive is deleted, the dive list model gets updated, the action becomes invisible, and then the delegate gets deleted as it is no longer needed (nothing to show, right?).
But the tap on the menu entry in the context menu caused a signal handler to execute (...onClicked()) that was running the onTriggered() signal for the deleteAction. So that signal handler isn't finished, because the onTriggered() function hasn't completed.
BOOM

This is rather convoluted and crazy, but I can convince myself that this makes sense. Of course, since I can reproduce the issue, I can't test my theory. So instead I created this rather interesting patch:

diff --git a/mobile-widgets/qml/DiveList.qml b/mobile-widgets/qml/DiveList.qml
index 7c04deddb..54723391b 100644
--- a/mobile-widgets/qml/DiveList.qml
+++ b/mobile-widgets/qml/DiveList.qml
@@ -278,11 +278,31 @@ Kirigami.ScrollablePage {
 		visible: currentItem && currentItem.myData && !currentItem.myData.isTrip
 		onTriggered: manager.toggleDiveInvalid(currentItem.myData.id)
 	}
+	Timer {
+		// this waits a tenth of a second and then deletes the dive
+		id: delayedDelete
+		interval: 100
+		onTriggered: {
+			manager.appendTextToLog("now deleting dive id " + diveToDelete)
+			manager.deleteDive(diveToDelete)
+		}
+	}
+	function deleteDive(diveId) { // this should be redundant, we should be able to start the timer from the onTriggered signal handler
+		diveToDelete = diveId
+		delayedDelete.start()
+		manager.appendTextToLog("triggered time to delete dive id " + diveToDelete)
+	}
+	property int diveToDelete: -1
 	property QtObject deleteAction: Kirigami.Action {
+		id: delAct
 		text: qsTr("Delete dive")
 		icon { name: ":/icons/trash-empty.svg" }
-		visible: currentItem && currentItem.myData && !currentItem.myData.isTrip
-		onTriggered: manager.deleteDive(currentItem.myData.id)
+		onTriggered: {
+			manager.appendTextToLog("calling the function to delete dive " + currentItem.myData.id)
+			deleteDive(currentItem.myData.id)
+			contextDrawer.close()
+			manager.appendTextToLog("closed drawer, done with the action")
+		}
 	}
 	property QtObject mapAction: Kirigami.Action {
 		text: qsTr("Show on map")


mind you, this of course creates its OWN race condition because we now have this 'diveToDelete' property sitting there and a timer that executes a delete on the ID in this property after 100ms.
I'd argue that if the user initiated this delete via the context menu it will take them a lot more than just 100ms after the context menu closes before they can initiate any other operation that would make this ID invalid - but that is not really an argument that I am super comfortable with.
This is, however, the easiest way I could think of to test my theory about what's happening.

So, Theodor, once .372 has been pushed out to the beta channel on Google Play, I'd love it if you could test that to see if you can still reproduce the crash...

fingers crossed

/D

> On Jan 19, 2021, at 1:21 PM, Dirk Hohndel via subsurface <subsurface at subsurface-divelog.org> wrote:
> 
> 
> 
>> On Jan 19, 2021, at 1:11 PM, Chirana Gheorghita Eugeniu Theodor <office at adaptcom.ro> wrote:
>> 
>> i can work on ssrftest... but i will need a pass also.
> 
> Ha - that's in the sources... it's 'geheim' (the German word for secret... yeah, my sense of humor is legendary)
> 
>> logs I get right after app crashes from the documents folder in the internal phone storage. That is where I find the log for subsurface. And after I send it I delete it so it gets recreated with new info of new tests. 
> 
> Actually, the app would recreate it anyway - but that's brilliant!
> 
>> Fot the builf part.... I think I can setup something. I work with nix* and compiling stuff for over 19 years now soo... android should not be very very difficult
> 
> If you have a Linux box that can run Docker, then it's super easy.  packaging/android/README  should explain it well, and if there are open questions, let's fix the documentation :-)
> 
>> I am surprised you cannot replicate. It's just adding a blank dive with no data, save and from list of dives screen delete from 3 dots or by long press.... One more thing i realize now: sometimes the delete option does not appear right away. I have to bring the menu second time up so I get the delete option. 
> 
> Trust me - I have tried this at least 50 times on half a dozen devices. I must be missing this one obvious thing.
> That's why I'm asking to see if you can reproduce it on an account that I also have access to.
> Next thing will be to ask you for a screen recording so I can EXACTLY replicate what you do (because different screen sizes can trigger slightly different behaviors, etc)
> 
>> I even tried now after saving the dive, exit app, relaunch it and delete. Delete option is there at first time but still crashes the app. but the empty dive is gone ... so it deletes something... next empty dive created gets dive number as normal ... every time after delete the new empty dive has the same number
>> Even cresting a second empty dive and deleting the first crashes the app... weird
> 
> And I absolutely cannot crash it. Not on Android 8, 10, 11, not on iOS.
> 
> /D
> _______________________________________________
> subsurface mailing list
> subsurface at subsurface-divelog.org
> http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface