Webots crashes for null radius in `Extrusion`

[ad-daniel]

Describe the Bug
As surfaced here #3627, when changing the radius of the signs (CautionSign, ExitSign,...) to 0 or to a negative value, it crashes Webots. In practice the problem likely stems from the Extrusion sub PROTO.

Although it's good practice to sanitize the parameters, it isn't normal that it crashes regardless.

It is not a regression.

[llessieux]

Is that one already fixed?
I am getting that message when I tried to change a CautionSign (yes it was a CautionSign and not a StopSign as noted below)
WARNING: StopSign (PROTO): Invalid 'radius' changed to 1. The value should be positive.
Tested on Master branch.

[ad-daniel]

Originally it was crashing on feature-enu-plan branch, however I just tested and it still crashes both in that branch and in master (ubuntu 20).

[omichel]

I could make it crash on Windows, by pressing the little "down" button several times until the value of radius reaches 0. But if I enter 0 or -1, it doesn't crash and I am getting the same warning as @llessieux.

[llessieux]

Indeed I can reproduce the crash that way too.
I might have a look at it next week too. Crashes tend to be easy to fix ;)

[llessieux]

I had a look at that and it is very strange. The checks are in place and it does prevent the value to go negative but somehow after the value is set to the default value of 1.0 it crashes elsewhere. I was also wondering why such tests were done so late in the process. By that I mean wouln't it be better to prevent the UI to try to set invalid values in the first place.

But it seems to be a problem caused by a re-entry loop inside the WbDoubleEditor when it is applying a value and a check is setting the value to something else than what was passed.

Basically things happen like that , the spin box changes the value to something below 0, the double editor notifies. it goes to the WbField in the Cylinder which checks the value and set it to 1. That triggers a regeneration, which triggers a clearSelection of the WbSceneTree. The clearSelection is calling the editField of the mFieldEditor with NULL.
This checks the current Editor and call applyIfNeeded. It goes back to the editor and check if the spin value is different from the mDouble (which has been reset to 1) so it tries to apply again and something goes wrong later.

Adding a mApplyingValue set to true when an apply call is in progress and back to false at the end and checking if that flag is set in applyIfNeeded and returning if it is already applying a value seems to fix it but I really wonder if that is the correct way.
This displays the warning in the console and the value is reset to 1 (instead of the default value specified in the Proto, I guess that one is lost).
I really feels that an editor should not have re-entering apply calls but I am not familiar with all use cases to say that it is the correct fix.
Now this probably should be applied to other editors too and perhaps factorized out in the WbValueEditor (somehow).

I can create a branch (master / develop?) to show the minimal fix then try to provide a more extensive solution that would cover all editors. (probably preApply/postApply/isApplying methods added to the WbValueEditor and called from all editors, with the inner code in apply in a try/catch to make sure that the postApply is called or if you have guard class to force operations on function exit use that.)

[omichel]

Thank you for the detailed report. Yes, you go ahead with a fix, preferably on the master branch. We will be happy to review it.

[llessieux]

Ok it doesn't seem to be as simple as I described it :(
My code prevents the first crash but clicking on the spin box again crashes in a completely different place.
And it doesn't seem to crash at always the same location either so some memory must be corrupted or something like that.
I will dig deeper but I have the impression that it is not just one bug ;)

[llessieux]

By the way, is there anyway to build Webots using Visual Studio?
The new one can debug with gdb information but it seems pretty slow to get the info.

[omichel]

By the way, is there anyway to build Webots using Visual Studio?

I never tried, but in theory, it should be possible.