trueagi-io / metta-wam

A Hyperon MeTTa Interpreter/Transpilier that targets the Warren Abstract Machine
8 stars 11 forks source link

Setup form in setup-call-cleanup! is not fully protected from interrupts #144

Open AdrickTench opened 3 weeks ago

AdrickTench commented 3 weeks ago

The prolog documentation for setup_call_cleanup/3 states

The execution of Setup is protected from asynchronous interrupts like call_with_time_limit/2 (package clib) or thread_signal/2.

I think it is most sensible if our implementation reliably does the same. I believe the bug arises when multiple results are at play.

Setup:

!(bind! &var (new-state 5)) ;; initialize higher than 1 for testing below
(= (increment)
   (sequential ((do (change-state! &var (+ 1 (get-state &var))))
               (get-state &var))))

Then this succeeds:

!(assertEqualToResult (catch (max-time! 1 
                                        (setup-call-cleanup! ((sleep! 2)
                                                              (change-state! &var 0))
                                                             ((increment) (increment)) ;; don't call this
                                                             (increment))) ;; do call this
                             time_limit_exceeded
                             (time_limit_exceeded (get-state! &var)))
                      ((time_limit_exceeded 1)))

But this fails, the only difference being the Setup form is wrapped in sequential:

!(assertEqualToResult (catch (max-time! 1 
                                        (setup-call-cleanup! (sequential ((sleep! 2)
                                                                          (change-state! &var 0)))
                                                             ((increment) (increment)) ;; don't call this
                                                             (increment))) ;; do call this
                             time_limit_exceeded
                             (time_limit_exceeded (get-state! &var)))
                      ((time_limit_exceeded 1)))

The intent of using sequential is to ensure that we sleep strictly before we (change-state! &var 0), to verify the interrupt doesn't prevent Setup from completing in full. My understanding is that execution order wouldn't be guaranteed for the non-sequential form across other MeTTa implementations that do more with concurrency (though it might happen to be guaranteed with Mettalog atm).