search

microsoft / pxt-microbit

A Blocks / JavaScript code editor for the micro:bit built on Microsoft MakeCode
https://makecode.microbit.org
Other
726 stars 638 forks source link

Memory leak in the play tone block with "looping in the background" #5640

Open microbit-carlos opened 6 months ago

microbit-carlos commented 6 months ago

Describe the bug When the play tone block is used with the "looping in the background" option selected, the micro:bit V1 and V2 run out of memory after a few seconds.

To Reproduce Steps to reproduce the behavior:

  1. Go to makecode.microbit.org
  2. Create a new project
  3. Drop the [play tone] block inside the forever loop
  4. Select the "looping in the background" option
  5. Flash the micro:bit
  6. After a couple of seconds the micro:bit panics with error 020 (out of memory)

https://makecode.microbit.org/_3VXXVo5Pjfgb

image

Expected behavior To not run out of memory

Screenshots N/A

micro:bit version (please complete the following information):

Tested with micro:bit V1 and V2.

Desktop (please complete the following information): N/A

Smartphone (please complete the following information): N/A

Additional context

makecode.microbit.org version: 6.0.26 Microsoft MakeCode version: 9.0.17 microbit runtime version: v2.2.0-rc6 codal-microbit-v2 runtime version: v0.2.63

Amerlander commented 6 months ago

I think starting background loops in a loop actually should result in out of memory.

But it could already drop a error in MakeCode, like this alternative version of the same a similar programm does: https://makecode.microbit.org/_UJkX9f9mqEP7 image

But these also does not show error messages: image image

abchatra commented 4 months ago

This is expected to throw the out of memory as the code is creating a looping tone forever in a loop.

We should just surface stack overflow error in the simulator if we can.

Why is this high priority?

jaustin commented 3 months ago

Firstly, I think you're right this isn't high priority - we'd mistriaged this thinking it was for all background melodies. Sorry, happy with it being descoped.

However, I don't think the OOM on the micro:bit is right? It appears as if there should only be one background melody: https://github.com/microsoft/pxt-microbit/blob/8862013b43c7f183a2c7943c11ce733c43c7a499/libs/core/music.ts#L407-L442

And indeed if you play a melody 'looping in background' and then a melody 'in background' the background loop stops.

image

https://makecode.microbit.org/_1TR5ehPt83xz

Given this, I think the bug to investigate here is "why does the real micro:bit OOM when you repeatedly play a melody in the background". It seems like it should just be replacing the previous background melody with the new one, and not causing stack overflow.