[PATCH] utils: Handle errors in worker threads.

[PATCH] utils: Handle errors in worker threads.

[Christopher Baines]

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

Re: [PATCH] utils: Handle errors in worker threads.

[Ludovic Courtès]

Hi,

Christopher Baines <mail@cbaines.net> 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’.

Re: [PATCH] utils: Handle errors in worker threads.

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 1806 bytes --]


Ludovic Courtès <ludo@gnu.org> writes:

> Hi,
>
> Christopher Baines <mail@cbaines.net> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]