parser goroutine leaks if not explicitly Stop()-ed

[XANi]

Currently not calling env.Stop() will make zygo not be GCed; code like this:

func worker() {
	env := zygo.NewZlisp()
	_ = env.LoadString("(+ 3 2)")
	_, _ = env.Run()
}

will quickly make it leak memory.

Maybe defer env.Stop() should be added somewhere in wiki ? Meanwhile I added some comment to the function (#51 ) so at least there is hint in godoc about.

[glycerine]

This is a reasonable concern. I've merged the Stop description addition.

Typically one only needs a single env for the process, so having the one parser background goroutine in the background is fine. It is like having an open file handle during the process.

But that isn't always true. Sometimes I end up with a bunch of parser goroutines just hanging around that won't be used in the future. A more complete solution would be to have method that is explicitly "OneShot" that automatically closes the parser goroutine after evaluating a string.

Or to have the parser goroutine timeout after a period of non-use.

Or to not spin up a seperate parser goroutine at all if there won't be multiple expressions evaluated. It is there to maintain state while parsing multiple lines pasted into a REPL.

I'll leave this open while thinking about what more to do.

[XANi]

Honestly I think just clarifying comments and documentation a bit would've been enough. I haven't found a mention about it running in the background and it wasn't in a example and not in IDE hint (as there was no comments) so I assumed it just get GCed as normal. RunAndClose() kind of method would provide way to run one-shots easily but considering there is already setup required anyway just asking user to call defer env.Close() isn't a big deal, especially that the Go stdlib does that in few places too.

Typically one only needs a single env for the process, so having the one parser background goroutine in the background is fine. It is like having an open file handle during the process.

My use case was using it as tiny query language so I did it per-call just to not have to do sync.lock->clear->run->unlock on each event

Or to have the parser goroutine timeout after a period of non-use.

That seems very "magical" and prone to errors. Like someone's code can pass all the tests then fail just because program "in real life" called the parser again after half a day. There's also runtime.SetFinalizer but as fair as I know it isn't exactly recommended, and it would really just hide bugs from user (unless maybe use it to print warning about unClose()d parser?)

[glycerine]

The latest release, v6.0.7, implements the ideas above in lazyparser.go, which was present before but not fully implemented. The LazyParse approach is now fully implemented and on by default. We start a parser goroutine only when needed, and shut it down eagerly. As a result, there should be no lingering goroutines if they are not being used. Calling Stop() is still a good idea, of course. I found having the extra goroutines in stack dumps a distraction. The LazyParse approach results in much cleaner stack dumps when the parsing goroutines are not in use, as they are no longer present.