Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
AdrickTench opened this issue Oct 4, 2024 · 0 comments
Labels
bug Issues that represent errors in the code
Milestone

Comments

@AdrickTench
Copy link
Collaborator

AdrickTench commented Oct 4, 2024

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).

@AdrickTench AdrickTench added the bug Issues that represent errors in the code label Oct 4, 2024
@TeamSPoon TeamSPoon added this to the Month 4 of 5 milestone Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues that represent errors in the code
Projects
None yet
Development

No branches or pull requests

2 participants