webui-dev / nim-webui

Use any web browser as GUI, with Nim in the backend and HTML5 in the frontend.
https://webui.me
MIT License
130 stars 9 forks source link

Some thought about crashes when calling js from Nim #15

Closed clzls closed 1 year ago

clzls commented 1 year ago

Similar to #5 , I went into some crashes when working with hello_world.nim using Nimble package. In devel mode it crashes silently when clicked, in release mode it works but sometimes it still crashes. I tried to change webui.nim:261 from

data = buffer.join().strip(leading = false, chars = {'\x00'})

to

data = $(cast[cstring](addr buffer[0]))

Then it seems to work smoothly, with thread:off. Not tested much and not using nightly build, but I want to note it here for others who is willing to make further investigations about it.

My env: Nim Compiler Version 1.6.12 [Windows: amd64]

neroist commented 1 year ago

Ah, thank you for this issue and the fix! I really haven't run into this issue, I wonder why that would cause an error 🤔

neroist commented 1 year ago

I've already opened a PR but I think it would be better if you opened one instead.

clzls commented 1 year ago

Ah, thank you for this issue and the fix! I really haven't run into this issue, I wonder why that would cause an error 🤔

I didn't know either since no exceptions or messages were shown (either Nim side or browser side) in my case, so I didn't open a PR at the first time...

As #17 has more details about the problem, I think we could close this one.

clzls commented 1 year ago

Some other thought: There are some calls returning webui-managed buffers, like encode. Would it be better if we copy the data to a Nim-managed string (by $) and destroy the returned buffers immediately? 😄

AlbertShown commented 1 year ago

I agree @clzl 👍 We should not forget to free the WebUI buffer using webui_free()

WebUI uses a custom memory management to allocate and free resources, so webui_free() will never cause a crash.

neroist commented 1 year ago

Some other thought: There are some calls returning webui-managed buffers, like encode. Would it be better if we copy the data to a Nim-managed string (by $) and destroy the returned buffers immediately? 😄

https://github.com/webui-dev/nim-webui/commit/b26d95c8b0246bc8a0ac89e31dee31ef7b748615 🎉