Re: [PATCH] utils: Handle errors in worker threads.
Ludovic Courtès writes: > Hi, > > Christopher Baines skribis: > >> Previously, if an error occurred, the worker fiber simply never sends a >> reply. In the case of HTTP requests to Cuirass, where an exception occurs >> when >> performing a database query, the fiber handling the request blocks as it >> never >> gets a response. I think that this has the potential to cause the process to >> hit file descriptor limits, as the connections are never responded to. >> >> This is fixed by responding with the details of the exception, and then >> throwing it within the fiber that made the call. >> >> * src/cuirass/utils.scm (make-worker-thread-channel): Catch exceptions when >> calling proc. >> (call-with-worker-thread): Handle receiving exceptions from the worker >> thread. > > Good catch! > >> + (put-message reply >> + (catch >> +#t > > Please put #t on the same line as ‘catch’. :-) Ok, I've made this change and pushed as bb225189fd56d89ec8be926dda269295ccbfe918. >> +(lambda () >> + (apply proc args)) >> +(lambda (key . args) >> + (cons* 'worker-thread-error key >> args)) > > As discussed with others at the Guix Days, it’s probably a good idea to > distinguish “remote” exceptions from local exceptions like you did (and > unlike what ‘inferior-eval’ does). > > LGTM! I don't quite follow what you're saying here, so feel free to elaborate, but it also didn't sound like there was a problem to fix, so I've gone ahead and pushed. Thanks for taking a look, Chris signature.asc Description: PGP signature
Re: [PATCH] utils: Handle errors in worker threads.
Hi, Christopher Baines skribis: > Previously, if an error occurred, the worker fiber simply never sends a > reply. In the case of HTTP requests to Cuirass, where an exception occurs when > performing a database query, the fiber handling the request blocks as it never > gets a response. I think that this has the potential to cause the process to > hit file descriptor limits, as the connections are never responded to. > > This is fixed by responding with the details of the exception, and then > throwing it within the fiber that made the call. > > * src/cuirass/utils.scm (make-worker-thread-channel): Catch exceptions when > calling proc. > (call-with-worker-thread): Handle receiving exceptions from the worker thread. Good catch! > + (put-message reply > + (catch > +#t Please put #t on the same line as ‘catch’. :-) > +(lambda () > + (apply proc args)) > +(lambda (key . args) > + (cons* 'worker-thread-error key > args)) As discussed with others at the Guix Days, it’s probably a good idea to distinguish “remote” exceptions from local exceptions like you did (and unlike what ‘inferior-eval’ does). LGTM! Ludo’.
[PATCH] utils: Handle errors in worker threads.
Previously, if an error occurred, the worker fiber simply never sends a reply. In the case of HTTP requests to Cuirass, where an exception occurs when performing a database query, the fiber handling the request blocks as it never gets a response. I think that this has the potential to cause the process to hit file descriptor limits, as the connections are never responded to. This is fixed by responding with the details of the exception, and then throwing it within the fiber that made the call. * src/cuirass/utils.scm (make-worker-thread-channel): Catch exceptions when calling proc. (call-with-worker-thread): Handle receiving exceptions from the worker thread. --- src/cuirass/utils.scm | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/cuirass/utils.scm b/src/cuirass/utils.scm index f3ba18d..4d59fec 100644 --- a/src/cuirass/utils.scm +++ b/src/cuirass/utils.scm @@ -114,7 +114,13 @@ arguments of the worker thread procedure." (let loop () (match (get-message channel) (((? channel? reply) . (? procedure? proc)) - (put-message reply (apply proc args + (put-message reply + (catch +#t +(lambda () + (apply proc args)) +(lambda (key . args) + (cons* 'worker-thread-error key args)) (loop))) (iota parallelism)) channel))) @@ -127,7 +133,10 @@ If already in the worker thread, call PROC immediately." (apply proc args) (let ((reply (make-channel))) (put-message channel (cons reply proc)) - (get-message reply) + (match (get-message reply) +(('worker-thread-error key args ...) + (apply throw key args)) +(result result)) (define-syntax-rule (with-worker-thread channel (vars ...) exp ...) "Evaluate EXP... in the worker thread corresponding to CHANNEL. -- 2.24.1