Page MenuHomePhabricator

[Segmentation] [Live Wire] Multiple reports of a crash due to image access out of bounds
Closed, ResolvedPublic

Description

I wasn't able to reproduce the issue but we had multiple reports of the same issue. Here's a screenshot of one reporter that indicates that it is somehow still possible to click outside of the image bounds which will lead to a crash.

Capture.PNG (290×363 px, 26 KB)

Maybe we should try to disable the bounds checking for clicks during debugging and make it robust enough to work/not crash in that situation, before enabling the bounds checking again.

Event Timeline

kislinsk triaged this task as Unbreak Now! priority.May 9 2023, 7:24 AM
kislinsk created this task.

I have the feeling it is an race condition/timing problem.
I was able to recreate the problem also in release after rapidly clicking arround (at least 50 times, machine gun style). In debug I clicked for minutes without anything appening.

And I am puzzled where in the EditableContour tool we have this boundary violation. May be when we generate the working slice. But the lasso tool which has also the problem according to one tester has no other image interactions...

I can now tell at least why you can paint into other windows.

The cause is that EditableContourTool::OnDrawing and EditableContourTool::OnEndDrawing do not check if they are still on the right plane.

OnStartDrawing does that and does an early out. BUT the state machine isn't aware of this and is now in the drawing state. So if you now move the mouse or release it on another window, the respective functions (see above) get triggered and start to work on another plane. (that most likely leads to the problem discribed in the task).

I don't have the time to take care of it. But I see to solutions.

Option 1: At state machine level
There was an option to also register conditions to transition. One could add a condition that checks for right plane and only allows the following transition if the condition is true.

<transition event_class="MouseMoveEvent" event_variant="PrimaryButtonMoved" target="Drawing" >
  <action name="Drawing"/>
</transition>

Option 2: At EditableContourTool
add checks to OnDrawing and OnEndDrawing if you are in the right plane.
But that is only be enough, if the following assumption is correct: It seems as the plane / render window is not changed if the state machine is active.
But has to be verified. First test indicates it.

Option 2 seems not to be enough, since I still can paint in other windows, even though it makes a little difference at least. Also, while free drawing a segment, you can leave a window and place a control point outside of the image which leads to the crashes reported.

I vote for revising the state machine and add conditions as you suggested in Option 1, and also add a check if still inside the image while drawing.