webui-dev / v-webui

Use any web browser as GUI, with V in the backend and modern web technologies in the frontend.
https://webui.me
MIT License
112 stars 11 forks source link

Fix `Collecting from unknown thread` GC error #62

Closed ttytm closed 1 year ago

ttytm commented 1 year ago

Fix based on the awesome help by @spytheman https://discordapp.com/channels/592103645835821068/592294828432424960/1151395814162321408

spytheman commented 1 year ago

Can we use something like this as a regression test?

import log
import time
import vwebui as ui

fn wait_condition(max_iterations int, sleep_duration time.Duration, cb fn() bool) bool {
    for i in 0 .. max_iterations {
        if cb() {
            return true
        }
        time.sleep(sleep_duration)
    }
    return false    
}

struct App {
mut:
    fn_was_called bool
}

fn allocate_lots_of_memory() {
    log.info('allocate_lots_of_memory start')
    for _ in 0..1000 { _ := 'abdef'.repeat(1000) }
    log.info('allocate_lots_of_memory end')
}

fn test_fn_call() {
    log.info('> ${@METHOD} start')
    defer {
        log.info('> ${@METHOD} end')
    }

    allocate_lots_of_memory()

    mut app := &App{}
    w := ui.new_window()
    defer {
        log.info('>> destroying window')
        w.destroy()
    }

    w.bind('v_fn', fn [mut app] (e &ui.Event) {
        log.info('>>> v_fn called')
        defer {
            log.info('>>> v_fn ended')
        }
        allocate_lots_of_memory()
        assert e.string() == 'foo'
        app.fn_was_called = true
    })

    w.show('<html style="background: #654da9; color: #eee">
<head><script src="/webui.js"></script></head>
<body>
    <samp>test_fn_call</samp>
    <script>setTimeout(async () => { await webui.call("v_fn", "foo"); }, 1000)</script>
</body>
</html>') or {
        assert false, 'Failed at w.show'
    }

    // Wait for the window to show
    ui.set_timeout(30)
    if !wait_condition(300, 100 * time.millisecond, fn [w] () bool { return w.is_shown() }) {
        assert false, 'Timeout showing window.'
    }
    log.info('> w.is_shown: ${w.is_shown()}')
    if !wait_condition(300, 100 * time.millisecond, fn [mut app] () bool { return app.fn_was_called }) {
        assert false, 'Timeout while waiting for the v binded callback to be called'
    }
}
spytheman commented 1 year ago

Note: I had to use w.show_browser(..., .chromium) locally, because it does not work with Firefox for now .

spytheman commented 1 year ago

With this PR, it prints:

#0 13:34:37 ᛋ master /v/vnew❱v z_test.v 
2023-09-13 13:34:42.433473 [INFO ] > test_fn_call start
2023-09-13 13:34:42.433516 [INFO ] allocate_lots_of_memory start
2023-09-13 13:34:42.441938 [INFO ] allocate_lots_of_memory end
2023-09-13 13:34:43.660647 [INFO ] > w.is_shown: true
2023-09-13 13:34:44.563163 [INFO ] >>> v_fn called
2023-09-13 13:34:44.563192 [INFO ] allocate_lots_of_memory start
2023-09-13 13:34:44.573315 [INFO ] allocate_lots_of_memory end
2023-09-13 13:34:44.573340 [INFO ] >>> v_fn ended
2023-09-13 13:34:44.665115 [INFO ] >> destroying window
2023-09-13 13:34:45.666423 [INFO ] > test_fn_call end
#0 13:34:45 ᛋ master /v/vnew❱

on master, it prints:

#0 13:34:03 ᛋ master /v/vnew❱v z_test.v 
2023-09-13 13:34:08.081091 [INFO ] > test_fn_call start
2023-09-13 13:34:08.081134 [INFO ] allocate_lots_of_memory start
2023-09-13 13:34:08.092042 [INFO ] allocate_lots_of_memory end
2023-09-13 13:34:09.312957 [INFO ] > w.is_shown: true
2023-09-13 13:34:10.227146 [INFO ] >>> v_fn called
2023-09-13 13:34:10.227174 [INFO ] allocate_lots_of_memory start
Collecting from unknown thread
#6 13:34:10 ᛋ master /v/vnew❱
ttytm commented 1 year ago

Can we use something like this as a regression test?

Very much so! To preserve the credits, you like to submit it in a PR?

spytheman commented 1 year ago

No, I do not want credits. I just want that the current PR is as complete as possible, so that it can detect regressions in the future.

spytheman commented 1 year ago

I would have added it in this PR, but I am not a maintainer here, so I can not edit/add more commits.

ttytm commented 1 year ago

Okay, just as fine. Thanks for the work @spytheman

I test how the test works in CI with a separate file, else I merge it with the current test. Splitting the tests and running them with xvfb and a browser on a GitHub runner made some issues. Things like VJOBS=1 didn't help.

ttytm commented 1 year ago

Okay works. I split the rest of the integration tests separately. Unnecessarily using ui.wait() was the culprit with splitting the tests.

ttytm commented 1 year ago

This was a crucial error and a something with high potential to be a blocker for the library. I'm very thankful for your important input @spytheman.

I had put another question if I should add you as co-author in a pair commit, but you already gave a no above. I also don't want to author things in your name. So all I can say is thanks for your efforts and the great insights :pray:

hassandraga commented 1 year ago

Thank you @spytheman for solving this issue. I appreciate it.

spytheman commented 1 year ago

Thank you for the amazing library. Using it with V is a joy, since it is very ergonomic.