Opened 12 years ago

Closed 4 months ago

Last modified 3 weeks ago

#8784 closed defect (fixed)

remove quit_sage() command from all.py top level

Reported by: was Owned by: jason
Priority: major Milestone: sage-9.6
Component: misc Keywords:
Cc: Merged in:
Authors: Michael Orlitzky Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: ef89f43 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description

It is stupid that it is this easy to accidentally destabilize and segfault Sage. Also, having a function "quit_sage()" available at the sage: prompt by default that does not quit sage, is dumb.

wstein@boxen:~/build/sage-4.4$ ./sage
----------------------------------------------------------------------
| Sage Version 4.4, Release Date: 2010-04-24                         |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------
sage: quit_sage()
Exiting Sage (CPU time 0m0.04s, Wall time 0m3.16s).
sage: quit
Exiting Sage (CPU time 0m0.07s, Wall time 0m4.80s).
/virtual/scratch/wstein/build/sage-4.4/local/bin/sage-sage: line 206: 11559 Segmentation fault      sage-ipython "$@" -i
wstein@boxen:~/build/sage-4.4$            

The fix is to rename quit_sage() somehow and change *all* code that calls it.

Change History (17)

comment:1 Changed 12 years ago by jason

Maybe rename it to "sage_library_cleanup"?

comment:2 Changed 9 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:3 Changed 9 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:4 Changed 8 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:5 Changed 8 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:6 Changed 7 months ago by mjo

  • Authors set to Michael Orlitzky
  • Branch set to u/mjo/ticket/8784
  • Commit set to b2d1032755b54c5624ea503e2cbbbc5a1dea981d
  • Status changed from new to needs_review

Now all of the cleanup happens automatically and quit_sage() is a no-op:

sage: quit_sage()
<ipython-input-1-ce1781e96a1f>:1: DeprecationWarning: quit_sage is deprecated and now does nothing; please simply delete it
See http://trac.sagemath.org/8784 for details.
  quit_sage()
sage:

New commits:

94dcf5cTrac #8784: clean up after symmetrica when sage exits.
7890d1bTrac #8784: clean up after m4ri when sage exits.
0cd365dTrac #8784: free the flint stack when sage terminates.
ebeb316Trac #8784: free the integer pool when sage exits.
b7ae30fTrac #8784: clear quaternion algebra globals when sage exits.
15182f2Trac #8784: cdef two internal quaternion functions.
eaf3978Trac #8784: quit pexpect processes when sage terminates.
b2d1032Trac #8784: deprecate quit_sage().

comment:7 Changed 7 months ago by mkoeppe

  • Milestone changed from sage-6.4 to sage-9.6

comment:8 follow-ups: Changed 6 months ago by dimpase

Does this mean that sage-cleaner may go, too?

comment:9 in reply to: ↑ 8 Changed 6 months ago by mjo

Replying to dimpase:

Does this mean that sage-cleaner may go, too?

Not yet, although that's another long-term goal of mine. sage-cleaner also cleans up "temporary" files, which it wouldn't have to do if we used the OS's built-in tempfile functions instead of our home-grown SAGE_TMP. The first and easiest part of that cleanup is #33213.

comment:10 in reply to: ↑ 8 Changed 6 months ago by mjo

Replying to dimpase:

Does this mean that sage-cleaner may go, too?

Ok, I actually did this in #33213, which now depends on this ticket.

comment:11 Changed 5 months ago by dimpase

  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to positive_review

OK, looks and tests good.

comment:12 Changed 5 months ago by dimpase

perhaps you want to rebase on the latest beta

comment:13 Changed 5 months ago by git

  • Commit changed from b2d1032755b54c5624ea503e2cbbbc5a1dea981d to ef89f43d374691171c23b8296e6737efcc72dedb
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:

7abf04dTrac #8784: clean up after symmetrica when sage exits.
3424071Trac #8784: clean up after m4ri when sage exits.
a053757Trac #8784: free the flint stack when sage terminates.
4f5a56dTrac #8784: free the integer pool when sage exits.
1bf07ffTrac #8784: clear quaternion algebra globals when sage exits.
1f86192Trac #8784: cdef two internal quaternion functions.
75e7ccbTrac #8784: quit pexpect processes when sage terminates.
ef89f43Trac #8784: deprecate quit_sage().

comment:14 Changed 4 months ago by dimpase

  • Status changed from needs_review to positive_review

oops, overlooked this, sorry. Looks good.

comment:15 Changed 4 months ago by vbraun

  • Branch changed from u/mjo/ticket/8784 to ef89f43d374691171c23b8296e6737efcc72dedb
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:16 Changed 4 months ago by mkoeppe

  • Commit ef89f43d374691171c23b8296e6737efcc72dedb deleted

Follow up in #33706.

comment:17 Changed 3 weeks ago by dimpase

We have a complaint about ipython processes staying alive in a patchbot. https://groups.google.com/d/msgid/sage-devel/20220717160807.GA13671%40metelu.net

Could it be due to this ticket, perhaps the change for src/sage/repl/ipython_extension.py was an overkill?

Note: See TracTickets for help on using tickets.