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

Already on GitHub? Sign in to your account

Workers are not terminated when no longer relevant. #331

Closed
iebuggy opened this Issue Jul 4, 2011 · 3 comments

Comments

Projects
None yet
3 participants
Contributor

iebuggy commented Jul 4, 2011

I've got a toolbar with a dropdown selection to change modes. Let's say I start off in PHP mode, switch temporarily to CSS mode, and then switch back to PHP mode. ACE loads the 'worker-css.js' file when CSS mode is selected and applies it to the current document. The worker, however, is not terminated when the mode changes to something else and continues to show "errors" in perfectly valid PHP code because it is attempting to interpret it as CSS. The demo is broken when switching the PHP document to CSS mode (Firefox reports an uncaught exception).
@gissues:{"order":63.97515527950327,"status":"backlog"}

Contributor

fjakobs commented Jul 5, 2011

Is this a problem? I have chosen to keep the worker around because the initialization is pretty expensive. You can manually activate and deactivate the worker though

Contributor

iebuggy commented Jul 5, 2011

The only alternative I see is to never allow workers in the first place in my code via setUseWorker(false). I see workers as a cool feature of ACE, but if this issue isn't fixable then I'll have no choice but to terminate all worker support.

Looking through the source, only Javascript seems to implement a terminate() method. But there also doesn't appear to be any way for any worker to ever actually pause or terminate. They all register for 'doc.on("change",...);' and always execute their validators. The simplest fix would perhaps to be to check for the mode of the session prior to calling 'worker.emit("change", e);' and compare it to the expected mode.

if (session.getMode() == ...) worker.emit("change", e);

Something like that. The only issue is if there is a deferred event still in the queue, it could still cause problems. Annotations should probably be cleared by setMode() as well. The this.$worker.terminate() call in this.$stopWorker can go away at that point since it doesn't actually seem to do what it implies it will do.

Edit: Well, actually thinking on this a bit more, getting rid of terminate() is probably the wrong approach. It looks like if I switch multiple times to a mode that has a worker, createWorker() is called every time that mode is selected. So this seems like a pretty big performance hit that stacks up (i.e. multiple 'doc.on("change", ...);'s will be called for the same document). I'm not sure who should be managing the worker if it is as big a performance hit as you say it is but the current approach is pretty much broken.

@nightwing nightwing closed this Oct 24, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment