Skip to content

Conversation

microbit-matt-hillsdon
Copy link
Contributor

@microbit-matt-hillsdon microbit-matt-hillsdon commented Sep 21, 2023

This fixes the example on #105.

I've added the debug script that I used, which I aspire to turn into an automated test in future.

The value does seem rather large but not really significant in a browser context. I've not explored whether it's possible to optimise our asyncify use.

I found an analogous issue in a JS interpreter here: justjake/quickjs-emscripten#112

@netlify
Copy link

netlify bot commented Sep 21, 2023

Deploy Preview for distracted-dubinsky-fd8a42 ready!

Name Link
🔨 Latest commit 8802c14
🔍 Latest deploy log https://app.netlify.com/sites/distracted-dubinsky-fd8a42/deploys/6512a245e5db2a00084c223d
😎 Deploy Preview https://deploy-preview-106--distracted-dubinsky-fd8a42.netlify.app/demo
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions
Copy link

@dpgeorge
Copy link
Collaborator

I use the following test to determine maximum stack depth when calling a function:

def g():
    global depth
    depth += 1
    g()
depth = 0
try:
    g()
except RuntimeError:
    pass
print('maximum recursion depth g():', depth)

That works only when MICROPY_STACK_CHECK is enabled, which it's not for this simulator. It might be worth trying to enable that option here, which would protect against large call depths.

Otherwise, you can make a simple fixed-depth test like this:

def f(n):
    if n:
        f(n - 1)
f(168)

Pick the first power of two where we fail with a pystack exhausted
error in MicroPython rather than an asyncify-related error.
@microbit-matt-hillsdon
Copy link
Contributor Author

microbit-matt-hillsdon commented Sep 26, 2023

I use the following test to determine maximum stack depth when calling a function:

def g():
    global depth
    depth += 1
    g()
depth = 0
try:
    g()
except RuntimeError:
    pass
print('maximum recursion depth g():', depth)

That works only when MICROPY_STACK_CHECK is enabled, which it's not for this simulator. It might be worth trying to enable that option here, which would protect against large call depths.

Otherwise, you can make a simple fixed-depth test like this:

def f(n):
    if n:
        f(n - 1)
f(168)

Yeah that makes sense. I got fixated on the structure of the original example. I've updated it to be a Python example alongside the others. Both examples you give work for me without enabling MICROPY_STACK_CHECK.

If I do enable MICROPY_STACK_CHECK (which is enabled for the CODAL port) I see worse behaviour:

FATAL: uncaught NLR 0x510d30 [firmware.js:1483:35](http://localhost:63072/build/firmware.js)
Aborted(Assertion failed: native function `stackRestore` called after runtime exit (use NO_EXIT_RUNTIME to keep it alive after main() exits)) [firmware.js:990:6](http://localhost:63072/build/firmware.js)
Aborted(Assertion failed: native function `mp_js_force_stop` called after runtime exit (use NO_EXIT_RUNTIME to keep it alive after main() exits)) [firmware.js:990:6](http://localhost:63072/build/firmware.js)
Simulator internal error: [demo:205:23](http://localhost:63072/demo)
Error: Aborted(Assertion failed: native function `mp_js_force_stop` called after runtime exit (use NO_EXIT_RUNTIME to keep it alive after main() exits))
    abort http://localhost:63072/build/firmware.js:1013
    assert http://localhost:63072/build/firmware.js:512
    createModule http://localhost:63072/build/firmware.js:1075
    forceStop http://localhost:63072/build/simulator.js:2201
    createRunningPromise http://localhost:63072/build/simulator.js:2470
    start http://localhost:63072/build/simulator.js:2440
    flash http://localhost:63072/build/simulator.js:2548
    createMessageListener http://localhost:63072/build/simulator.js:2751
    EventListener.handleEvent* http://localhost:63072/build/simulator.js:2826
    <anonymous> http://localhost:63072/build/simulator.js:2827
[demo:206:23](http://localhost:63072/demo)

Some of that (the mp_js_force_stop bit at least) seems like it might be a knock-on error from whatever originally goes wrong.

For the moment at least, I think it makes sense to go with the PR as I've just updated it.

</option>
<option value="sound_effects_user">Sound effects (user)</option>
<option value="speech">Speech</option>
<option value="stack_size">Stack size</option>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adds the example to the list in the demo

image

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, very nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants