Fix memory leak #269
Fix memory leak #269
Conversation
crux_core/src/capability/executor.rs
Outdated
| // we read off both queues and execute the tasks we receive. | ||
| // Since either queue can generate work for the other queue, | ||
| // we read from them in a loop until we are sure both queues | ||
| // are exhaused | ||
| let mut did_some_work = true; |
There was a problem hiding this comment.
Would it be simpler to automatically consider received tasks ready and enqueue them as they get an ID? Here we would then go through all new tasks first, then run the ready queue to exhaustion?
Unless I'm missing something, which is not unlikely :)
There was a problem hiding this comment.
Oh, what I'm probably missing is that ready tasks may spawn further tasks, which we need to take in. So the result is the same tik-tok loop...
There was a problem hiding this comment.
Yeah it's an ugly loop but necessary, I think
There was a problem hiding this comment.
There is a chance this might alter observable behaviour, since the running order of the tasks has potentially changed. Of course we shouldn't be relying on the order that effects arrive, but in practice at least in tests it is hard to avoid.
That could be avoided by having everything arrive on one queue (using an enum), not very pretty though.
There was a problem hiding this comment.
I think effectively random order of execution is a good thing - nothing should be relying on the ordering (beyond the causal one).
adwhit
force-pushed
the
alex/memory-leak
branch
from
be4f11b to
48f6c15
Compare
I made this primarily to show how it can be done, rather than to offer as the 'right' fix. Basically instead of passing
Arc<Task>everywhere we keep the Task inside theQueuingExecutorand just send auuidu32to notify when it should be polled again. Very 'async executor 101'.Fixes #268
TODO: Update runtime docs