bigtestjs / server

All BigTest development has moved to https://github.com/thefrontside/bigtest
https://github.com/thefrontside/bigtest
2 stars 1 forks source link

Prevent listener leaks, closes #67 #69

Closed jnicklas closed 4 years ago

jnicklas commented 4 years ago

Essentially this reverts watch to work more like it did before, where it only ever creates one listener, thus preventing leaks.

I'm honestly not too sure why the listeners were leaking in the other implementation, I think it shouldn't be happening, but it might be some combination of this implementation using monitor and on together?

At any rate, this implementation is probably more performant, and easier to reason about, since we're not attaching and removing listeners all the time.

The downside is that we have to use ensure to get rid of the listener. We could use a monitor, but that seems like a bit on an anti pattern to me. We definitely want the detachment of the listener to be synchronous with shut-down of the Execution, so I think using ensure is better.