Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix example to clean up watchdog goroutine #431

Merged
merged 5 commits into from
Nov 29, 2022
Merged

fix example to clean up watchdog goroutine #431

merged 5 commits into from
Nov 29, 2022

Conversation

raidancampbell
Copy link
Contributor

This fixes the example code for cleaning up long-running VMs. Previously, they were killed after the timeout: the goroutine tracking this timeout holds a reference to the VM, therefore keeping the entire VM in memory for the duration of the configured timeout regardless of whether it has already completed its execution.

This change adds a second layer of communication to the watchdog goroutine, so that it will terminate itself if the VM has exited within the timeout. This allows the VM to be garbage collected if no other references to the VM exist.

README.markdown Outdated Show resolved Hide resolved
README.markdown Outdated Show resolved Hide resolved
Copy link
Collaborator

@stevenh stevenh left a comment

Choose a reason for hiding this comment

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

Thanks for this, in case its easier this is the full block with formatting fixed:

    vm := otto.New()
    vm.Interrupt = make(chan func(), 1) // The buffer prevents blocking
    watchdogCleanup := make(chan struct{})
    defer close(watchdogCleanup)

    go func() {
        select {
        case <-time.After(2 * time.Second): // Stop after two seconds
            vm.Interrupt <- func() {
                panic(halt)
            }
        case <-watchdogCleanup:
        }
        close(vm.Interrupt)
    }()

README.markdown Outdated Show resolved Hide resolved
README.markdown Outdated Show resolved Hide resolved
README.markdown Outdated Show resolved Hide resolved
raidancampbell and others added 3 commits November 28, 2022 10:23
Co-authored-by: Steven Hartland <steven.hartland@multiplay.co.uk>
Co-authored-by: Steven Hartland <steven.hartland@multiplay.co.uk>
Co-authored-by: Steven Hartland <steven.hartland@multiplay.co.uk>
@raidancampbell
Copy link
Contributor Author

heh, thanks for making my life easy by turning those into suggested changes: all have been applied

@stevenh stevenh merged commit 4617108 into robertkrimen:master Nov 29, 2022
sg3des pushed a commit to sg3des/otto that referenced this pull request Jul 17, 2023
Fix the example to clean up watchdog goroutine after either the timeout or the VM completes.

Co-authored-by: Steven Hartland <steven.hartland@multiplay.co.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants