google / blockly-android

Blockly for Android
Apache License 2.0
671 stars 209 forks source link

Nesting 40+ blocks causes a StackOverflowException on dalvik #179

Open RoboErikG opened 8 years ago

RoboErikG commented 8 years ago

On older devices that run dalvik instead of art we have a much smaller stack limit. This causes an overflow when the spaghetti test is run on devices running Dalvik.

For a short term solution we should consider not allowing connections that increase the stack depth beyond some limit.

Longer term we can consider maintaining the measure/layout dependencies separately and flattening the view hierarchy.

AnmAtAnm commented 8 years ago

Short term solution: Add two block stack depth limits to the Blockly configuration, for art and for dalvik. Users actions that hit their respective stack limit should inform the user the action isn't allowed. This should include loading a file that has a excessive stack depth, and appending two long block chains together. Write some docs describe the difference, and encourage app developers that share files between devices to set both values to the smaller of the two.

AnmAtAnm commented 7 years ago

ART was introduced in Kitkat, and so Dalvik issues should be limited to Jellybean, for our purposes. As of June 5th dashboards, the Jellybean is approximately 8.8% of the market (with 1.6% being even lower), with 7.5% being APIs 16 and 17. Unfortunately, we have other standing issues (#537 and #540) that have pushed us to unsupport those two versions. While that was intended to be temporary, we have not had any complaints since doing so. This leaves the remaining 1.3% Android users as potentially affected by this already rare Blockly issues. Given this, I am going to deprioritize this bug until/unless #537 and #540 are fixed and we support APIs down to 16 again.

AnmAtAnm commented 7 years ago

Correction: ART was only experimental in KitKat and so most of that 18.1% is probably still on Dalvik.

AnmAtAnm commented 6 years ago

Reported in #681, the effective input depth is 6 blocks on dalvik machines. The user reported crash on the 8th block, but in my emulator tests it crashes on the 7th block. This is much lower than the earlier estimate. We probably grew the method call per block since the original report.

Nested statement inputs crash on the 9th inner block.