Corona runtime crash: display.remove(Group), display.getCurrentStage():setFocus(objects within the Group) clashing

paolo.longato · May 2, 2013, 12:19pm

Please take a look at the following code. I create a rectangle (i.e. a button), put it inside a group and add a touch listener to it. Inside the touch listener I set the focus to the objects touched. Then I use a timer to remove the group within 3 seconds of being created.

If I am still pushing the “button” past the 3 seconds, when I then release I obtain that both the Corona simulator and the device crash. If I don’t set focus to the object touched OR I remove the button directly (and not its parent group) then no crash occurs.

Is this documented somewhere or have I hit a bug? What is actually going on in the event dispatch system? Is there some solution that does not require looping inside each layer of my display groups in order to remove a custom button/touch-sensitive-object?

Any indication welcome.

***

Run the following code in a main.lua on simulator, press the white button for above 3 seconds and then release. This should crash the simulator (and the device if you care to build).

\_W = display.contentWidth \_H = display.contentHeight local objectsB = display.newGroup() local button = display.newRect(\_W/2-100, \_H/2-100, 200, 200) objectsB:insert(button) -- \<== causing crash local function onTouch(event) self = event.target if event.phase == "began" or event.phase == "moved" then display.getCurrentStage():setFocus(self) -- \<== causing crash self:setFillColor(100,100,100) elseif event.phase == "ended" then self:setFillColor(255,255,255) display.getCurrentStage():setFocus(nil) -- \<== causing crash end return true end button:addEventListener("touch", onTouch) local function delay() display.remove(objectsB) -- \<== causing crash objectsB = nil -- causing crash end timer.performWithDelay(3000, delay)

aukStudios · May 2, 2013, 1:28pm

Interesting. I’m able to reproduce the crash you’re seeing. It goes away if you insert [lua]button:removeEventListener(“touch”, onTouch)[/lua] as the first line of your delay function, but I don’t see why that should be necessary. I’d suggest filing a bug report.

Andrew

paolo.longato · May 2, 2013, 1:36pm

@Andrew, that is precisely my point. It should not be necessary. It also goes away in three other occasions:

(1) get rid of setFocus()

(2) remove the object separately before removing the group

(3) display.getCurrentStage():setFocus(nil) BEFORE removing the group

(4) the one that you suggest: remove the listener separately

Can you fix the issue using (3)? If (3) is a workaround, it should be, at the very least, documented as a best practice when cleaning a scene.

paolo.longato · May 2, 2013, 1:56pm

Crash report – Case 23205

aukStudios · May 2, 2013, 2:00pm

Yup, #3 fixes it for me too.

Andrew

richard9 · May 2, 2013, 2:57pm

Can’t guarantee this isn’t a bug but there is some fishy things in the code to my eyes…

You’re setting focus on every began/moved frame.
You’re not releasing focus when the button is removed, which means you could easily be in the middle of a moved frame and trying to execute code when the button was just removed.

Realistically the code should be something more like this, just a guess…

\_W = display.contentWidth \_H = display.contentHeight local objectsB = display.newGroup() local button = display.newRect(\_W/2-100, \_H/2-100, 200, 200) objectsB:insert(button) local function onTouch(self, event) if event.phase == "began" or event.phase == "moved" and not self.isFocus then display.getCurrentStage():setFocus(self) self.isFocus = true self:setFillColor(100,100,100) elseif event.phase == "ended" and self.isFocus then display.getCurrentStage():setFocus(nil) self.isFocus = false self:setFillColor(255,255,255) end return true end button.touch = onTouch button:addEventListener("touch") local function delay() display.getCurrentStage():setFocus(nil) display.remove(objectsB) objectsB = nil end timer.performWithDelay(3000, delay)

paolo.longato · May 3, 2013, 8:12am

@richard9 – Thanks for you great feedback

(1) – Setting the focus at each touch “began” phase is ok because focus on the button is always off before user touches the screen; BUT you have a point on the “moved” phase, no need to set the focus again and again. Having said that I am not detecting meaningful inefficiency or errors by doing that. I will surely follow your best practice from now on.

(2) is also true, but if you remove the object directly (the “button” in my code) you will see that you do not need to reset focus before removing the object: all listeners are supposed to stop before an object is removed and that is what happens indeed. The problem arises when an object is part of a group and only the group is removed: in that case events are still dispatched to the non existing object causing a crash with signal 11 (on iOS). Given that this is not consistent behaviour I have filed a bug report. Corona has told me that they already had a similar bug report.

As you pointed out, the workaround is easy: make sure that focus is released before you remove a group that has objects with touch listeners (line 26 of your code). Having said that (and I might be wrong here) the problem I see with the workaround is that you might have instances when you want to keep the focus and still dynamically remove groups with objects/touch-listeners inside (e.g. focus might be on another object or group according to what user does). In that case I think you need to loop through your object/touch-listeners and figure out which one has a property .isFocus == true. Alternatively Corona could update their display.remove(<group>) API in order to be consistent with display.remove(<object>).

paolo.longato · May 3, 2013, 8:13am

@richard9 – Thanks for you great feedback

(1) – Setting the focus at each touch “began” phase is ok because focus on the button is always off before user touches the screen; BUT you have a point on the “moved” phase, no need to set the focus again and again. Having said that I am not detecting meaningful inefficiency or errors by doing that. I will surely follow your best practice from now on.

(2) is also true, but if you remove the object directly (the “button” in my code) you will see that you do not need to reset focus before removing the object: all listeners are supposed to stop before an object is removed and that is what happens indeed. The problem arises when an object is part of a group and only the group is removed: in that case events are still dispatched to the non existing object causing a crash with signal 11 (on iOS). Given that this is not consistent behaviour I have filed a bug report. Corona has told me that they already had a similar bug report.

As you pointed out, the workaround is easy: make sure that focus is released before you remove a group that has objects with touch listeners (line 26 of your code). Having said that (and I might be wrong here) the problem I see with the workaround is that you might have instances when you want to keep the focus and still dynamically remove groups with objects/touch-listeners inside (e.g. focus might be on another object or group according to what user does). In that case I think you need to loop through your object/touch-listeners and figure out which one has a property .isFocus == true. Alternatively Corona could update their display.remove(<group>) API in order to be consistent with display.remove(<object>).

aukStudios · May 2, 2013, 1:28pm

Interesting. I’m able to reproduce the crash you’re seeing. It goes away if you insert [lua]button:removeEventListener(“touch”, onTouch)[/lua] as the first line of your delay function, but I don’t see why that should be necessary. I’d suggest filing a bug report.

Andrew

paolo.longato · May 2, 2013, 1:36pm

@Andrew, that is precisely my point. It should not be necessary. It also goes away in three other occasions:

(1) get rid of setFocus()

(2) remove the object separately before removing the group

(3) display.getCurrentStage():setFocus(nil) BEFORE removing the group

(4) the one that you suggest: remove the listener separately

Can you fix the issue using (3)? If (3) is a workaround, it should be, at the very least, documented as a best practice when cleaning a scene.

paolo.longato · May 2, 2013, 1:56pm

Crash report – Case 23205

aukStudios · May 2, 2013, 2:00pm

Yup, #3 fixes it for me too.

Andrew

richard9 · May 2, 2013, 2:57pm

Can’t guarantee this isn’t a bug but there is some fishy things in the code to my eyes…

You’re setting focus on every began/moved frame.
You’re not releasing focus when the button is removed, which means you could easily be in the middle of a moved frame and trying to execute code when the button was just removed.

Realistically the code should be something more like this, just a guess…

\_W = display.contentWidth \_H = display.contentHeight local objectsB = display.newGroup() local button = display.newRect(\_W/2-100, \_H/2-100, 200, 200) objectsB:insert(button) local function onTouch(self, event) if event.phase == "began" or event.phase == "moved" and not self.isFocus then display.getCurrentStage():setFocus(self) self.isFocus = true self:setFillColor(100,100,100) elseif event.phase == "ended" and self.isFocus then display.getCurrentStage():setFocus(nil) self.isFocus = false self:setFillColor(255,255,255) end return true end button.touch = onTouch button:addEventListener("touch") local function delay() display.getCurrentStage():setFocus(nil) display.remove(objectsB) objectsB = nil end timer.performWithDelay(3000, delay)

paolo.longato · May 3, 2013, 8:12am

@richard9 – Thanks for you great feedback

(1) – Setting the focus at each touch “began” phase is ok because focus on the button is always off before user touches the screen; BUT you have a point on the “moved” phase, no need to set the focus again and again. Having said that I am not detecting meaningful inefficiency or errors by doing that. I will surely follow your best practice from now on.

(2) is also true, but if you remove the object directly (the “button” in my code) you will see that you do not need to reset focus before removing the object: all listeners are supposed to stop before an object is removed and that is what happens indeed. The problem arises when an object is part of a group and only the group is removed: in that case events are still dispatched to the non existing object causing a crash with signal 11 (on iOS). Given that this is not consistent behaviour I have filed a bug report. Corona has told me that they already had a similar bug report.

As you pointed out, the workaround is easy: make sure that focus is released before you remove a group that has objects with touch listeners (line 26 of your code). Having said that (and I might be wrong here) the problem I see with the workaround is that you might have instances when you want to keep the focus and still dynamically remove groups with objects/touch-listeners inside (e.g. focus might be on another object or group according to what user does). In that case I think you need to loop through your object/touch-listeners and figure out which one has a property .isFocus == true. Alternatively Corona could update their display.remove(<group>) API in order to be consistent with display.remove(<object>).

paolo.longato · May 3, 2013, 8:13am

@richard9 – Thanks for you great feedback

(1) – Setting the focus at each touch “began” phase is ok because focus on the button is always off before user touches the screen; BUT you have a point on the “moved” phase, no need to set the focus again and again. Having said that I am not detecting meaningful inefficiency or errors by doing that. I will surely follow your best practice from now on.

(2) is also true, but if you remove the object directly (the “button” in my code) you will see that you do not need to reset focus before removing the object: all listeners are supposed to stop before an object is removed and that is what happens indeed. The problem arises when an object is part of a group and only the group is removed: in that case events are still dispatched to the non existing object causing a crash with signal 11 (on iOS). Given that this is not consistent behaviour I have filed a bug report. Corona has told me that they already had a similar bug report.

As you pointed out, the workaround is easy: make sure that focus is released before you remove a group that has objects with touch listeners (line 26 of your code). Having said that (and I might be wrong here) the problem I see with the workaround is that you might have instances when you want to keep the focus and still dynamically remove groups with objects/touch-listeners inside (e.g. focus might be on another object or group according to what user does). In that case I think you need to loop through your object/touch-listeners and figure out which one has a property .isFocus == true. Alternatively Corona could update their display.remove(<group>) API in order to be consistent with display.remove(<object>).