shikokuchuo / mirai

mirai - Minimalist Async Evaluation Framework for R
https://shikokuchuo.net/mirai/
GNU General Public License v3.0
193 stars 10 forks source link

stop_mirai behavior #110

Closed jcheng5 closed 7 months ago

jcheng5 commented 7 months ago

In the task APIs I've used in the past, task cancellation is usually just another kind of error. (java.util.concurrent.CancellationException , PTHREAD_CANCELED) So I found the stop_mirai behavior surprising, specifically the part with my emphasis:

Stops the asynchronous operation associated with the mirai by aborting, and then waits for it to complete or to be completely aborted. The mirai is then deallocated and attempting to access the value at $data will result in an error.

This is surprising to me because when a mirai is interrupted or errors, then $data returns a specially classed value. Is there something fundamentally different about the way you intended stop_mirai to be used?

The reason I ask is because I'm trying to implement a Shiny example app that allows you to start and cancel a mirai object through ExtendedTask. My attempt is here: https://gist.github.com/jcheng5/1283baec96c05a65778d931a8b7c7314 But it fails because (I believe) this unresolved() call errors.

(Or maybe stop_mirai wasn't intended to be used for cancellation/interruption, just for cleanup during process shutdown? I would definitely understand that...)

shikokuchuo commented 7 months ago

stop_mirai() was never a core part of the API in the way say call_mirai() is.

I guess the thinking was not to 'encourage' its use as it only aborts the receipt of the task in hand - the task could still be ongoing on a daemon. In this way, it's not necessarily a free lunch to start and then stop a lot of tasks.

For interactive use, it's obvious that after calling stop_mirai(), there is no point in trying to access it. However, for programmatic use, I see your point, and I don't see a fundamental hurdle to returning a classed error. In fact, I can change it to return a 'miraiInterrupt', as if the mirai had been interrupted. The example you were attempting should then work.

shikokuchuo commented 7 months ago

Now implemented in mirai 0.13.2.9001, with more precise documentation wording. Regardless of whether it still fits the use case you had in mind, I do like how it is at least consistent now in returning an error value - thanks for pointing this out.

shikokuchuo commented 7 months ago

Do feel free to re-open if you think something else would work better.

jcheng5 commented 7 months ago

Oh, I just re-read your comment and see that it doesn't actually stop the work... yeah, I don't think I can recommend that then. The "standard" way to do this AFAIK is cooperative cancellation, where the task logic needs to explicitly and frequently check whether it's been cancelled. https://learn.microsoft.com/en-us/dotnet/api/system.threading.cancellationtoken?view=net-8.0

jcheng5 commented 7 months ago

Hmmm, or in the case of R, maybe sending SIGINT/Ctrl+C is more appropriate.

shikokuchuo commented 7 months ago

I re-examined how 'aio' cancellation is handled at nanonext and confirm that this really hasn't been touched since the very early days. At that time, I think for safety, I had to remove access to the external pointer after calling this function, and that's why it operated a bit differently.

However, I've now updated it to how it should operate, which is to resolve to an 'errorValue' 20 (operation cancelled). It waits until the local operation is completely cancelled and the completion callback run (and hence any pending promises).

This doesn't address the cancellation of ongoing tasks on daemons, but now presents a good 'baseline' behaviour (in builds 9008 of nanonext/mirai).