Opened 3 years ago

Last modified 2 years ago

#27428 new defect

Invalid use of sig_on() in acb_calc_func_callback

Reported by: jdemeyer Owned by:
Priority: major Milestone:
Component: cython Keywords:
Cc: mmezzarobba Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by 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). 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).

Change History (10)

comment:1 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:2 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:3 in reply to: ↑ description ; follow-up: Changed 3 years ago by mmezzarobba

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?

comment:4 in reply to: ↑ 3 Changed 3 years ago by jdemeyer

  • Description modified (diff)

Replying to mmezzarobba:

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

comment:5 Changed 3 years ago by gh-Hrishabh-yadav

  • Branch set to u/gh-Hrishabh-yadav/filter_kruskal_spanning_tree

comment:6 follow-up: Changed 3 years ago by jdemeyer

  • Branch u/gh-Hrishabh-yadav/filter_kruskal_spanning_tree deleted

Wrong ticket?

comment:7 in reply to: ↑ 6 ; follow-up: Changed 3 years ago by gh-Hrishabh-yadav

Replying to jdemeyer:

Wrong ticket?

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

comment:8 in reply to: ↑ 7 Changed 3 years ago by gh-Hrishabh-yadav

Replying to gh-Hrishabh-yadav:

Replying to jdemeyer:

Wrong ticket?

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

Thanks

comment:9 Changed 3 years ago by embray

  • Milestone changed from sage-8.7 to sage-8.8

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

comment:10 Changed 2 years ago by embray

  • Milestone sage-8.8 deleted

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

Note: See TracTickets for help on using tickets.