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

Invalid use of sig_on() in acb_calc_func_callback #27428

Open
jdemeyer opened this issue Mar 5, 2019 · 12 comments
Open

Invalid use of sig_on() in acb_calc_func_callback #27428

jdemeyer opened this issue Mar 5, 2019 · 12 comments

Comments

@jdemeyer
Copy link

jdemeyer commented Mar 5, 2019

The function acb_calc_func_callback ends with a sig_on() statement. This makes absolutely no sense at all (see https://cysignals.readthedocs.io/en/latest/interrupt.html#using-sig-on-and-sig-off). This was introduced in #24686.

In that same function, there is also a bare except: which should be fixed to except BaseException: (you really want to catch all exceptions, so it's better to be explicit about that).

CC: @mezzarobba

Component: cython

Issue created by migration from https://trac.sagemath.org/ticket/27428

@jdemeyer jdemeyer added this to the sage-8.7 milestone Mar 5, 2019
@jdemeyer

This comment has been minimized.

@jdemeyer

This comment has been minimized.

@mezzarobba
Copy link
Member

Replying to @jdemeyer:

The function acb_calc_func_callback ends with a sig_on() statement. This makes absolutely no sense at all (see https://cysignals.readthedocs.io/en/latest/interrupt.html#using-sig-on-and-sig-off).

Thanks for the notice. I think I'm unable to fix the issue by myself, unfortunately.

Is there a description of sig_on()/sig_off() that explains what they do and where the rules listed in the cysignal manual come from, instead of just saying what one should and shouldn't do? I don't remember the rule that “sig_off() should be called before the function that called sig_on() returns” being there when we wrote that code (but I may well have missed it).

What is the proper way to handle the case of (i) an external C library (ii) that performs callbacks into Python code (iii) but not very frequent ones, so that one would like the code running between two callbacks to be interruptible?

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

jdemeyer commented Mar 5, 2019

comment:4

Replying to @mezzarobba:

Is there a description of sig_on()/sig_off() that explains what they do and where the rules listed in the cysignal manual come from

Not really, but the important bit to know is that it relies on setjmp()/longjmp(). So the restriction comes from setjmp(). From the setjmp man page:

If the function which called setjmp() returns before longjmp() is called, the behavior is undefined.

I don't remember the rule that “sig_off() should be called before the function that called sig_on() returns” being there when we wrote that code

How old is your code? That piece of documentation dates from #10109 :-)

What is the proper way to handle the case of (i) an external C library (ii) that performs callbacks into Python code (iii) but not very frequent ones, so that one would like the code running between two callbacks to be interruptible?

It's complicated. The hardest part is dealing with exceptions. I see that you already have provisions for that using the ctx object. An alternative way for dealing with exceptions is sig_error(). Note that you still have a lot of Python code unguarded by try/except which could raise exceptions (even an assert statement!).

The easy part is making sure that interrupts cannot happen while executing Python code. That can be done with sig_block()/sig_unblock().

@Hrishabh-yadav
Copy link
Mannequin

Hrishabh-yadav mannequin commented Mar 21, 2019

@jdemeyer
Copy link
Author

comment:6

Wrong ticket?

@jdemeyer
Copy link
Author

Changed branch from u/gh-Hrishabh-yadav/filter_kruskal_spanning_tree to none

@Hrishabh-yadav
Copy link
Mannequin

Hrishabh-yadav mannequin commented Mar 21, 2019

comment:7

Replying to @jdemeyer:

Wrong ticket?

I know, It was a mistake.. How do I remove a branch from a ticket

@Hrishabh-yadav
Copy link
Mannequin

Hrishabh-yadav mannequin commented Mar 21, 2019

comment:8

Replying to @Hrishabh-yadav:

Replying to @jdemeyer:

Wrong ticket?

I know, It was a mistake.. How do I remove a branch from a ticket

Thanks

@embray
Copy link
Contributor

embray commented Mar 25, 2019

comment:9

Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)

@embray embray modified the milestones: sage-8.7, sage-8.8 Mar 25, 2019
@embray
Copy link
Contributor

embray commented Jun 14, 2019

comment:10

As the Sage-8.8 release milestone is pending, we should delete the sage-8.8 milestone for tickets that are not actively being worked on or that still require significant work to move forward. If you feel that this ticket should be included in the next Sage release at the soonest please set its milestone to the next release milestone (sage-8.9).

@embray embray removed this from the sage-8.8 milestone Jun 14, 2019
mezzarobba added a commit to mezzarobba/sage that referenced this issue Feb 9, 2023
using undocumented cysignal functions sig_block()/sig_unblock(), based
on Jeroen Demeyer's indications[1]

[1] sagemath#27428 (comment)
mezzarobba added a commit to mezzarobba/sage that referenced this issue Feb 10, 2023
using undocumented cysignal functions sig_block()/sig_unblock(), based
on Jeroen Demeyer's indications[1]

[1] sagemath#27428 (comment)
vbraun pushed a commit that referenced this issue Mar 19, 2023
    

    
URL: #35044
Reported by: Marc Mezzarobba
Reviewer(s): Frédéric Chapoton
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants