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
Trying completion list of maxima_lib crashes Sage #22766
Comments
comment:1
I think the crash happens in line |
comment:3
On OS X (10.12.6, Xcode 8.3.3, Sage 8.1.beta6), I don't get this crash, and I also don't see the message "Building Maxima command completion list ...": I just immediately get a list of completions. |
comment:4
Replying to @jhpalmieri:
It's because you already have this list built and stored in somewhere in |
comment:5
Okay, I get the crash after deleting |
comment:6
Replying to @jhpalmieri:
Yes, it is because in this case everything happens in the same thread (the one of the tab completion). |
comment:7
Making maxima_lib "thread-safe" would consist of locking it to one thread. Due to the signal management switching that happens upon entering/exiting ecllib makes it fundamentally incompatible with multi-threading, because signal handlers are process-specific; not thread-specific. Sage installs special signal handlers (for SIGINT, for instance), and so does ECL. If ECL runs with multi-threading, ECL even goes further with signal handling (it makes a dedicated signal handling thread), and it uses signals to synchronize threads for critical GC operations. If you want to get ecllib to a state where it can safely be used in a multi-threaded environment, I think one would have to unify the signal management of sage and ecl. The result would not actually make maxima_lib threadsafe, because maxima itself is rather fundamentally not thread-safe. |
comment:8
Removing dependencies seems so much more promising, see https://trac.sagemath.org/wiki/symbolics/maxima |
comment:9
Here is another scenario with two threads, only leading to an abort, not to a segfault.
with the following prints inserted into
|
comment:10
I think what we see here is ECL being initialised in a thread (number 32) that later is shut down, and then It seems that indeed we must make sure that ECL is always started in the main thread, which does not disappear. |
comment:11
A part of the relevant discussion is on #23956, which I'll close as duplicate. |
Dependencies: #23956 |
This comment has been minimized.
This comment has been minimized.
comment:14
I wonder whether any other extension (apart from ECL/Maxima) is affected by this issue. |
comment:15
Reiterating from 23956: The effect of That means it is NOT safe to execute sage code in a thread parallel to a thread that is executing ecl code (properly). So, if we allow for multiple threads in sage, we'd strictly have to halt all the other threads upon executing In addition, we're running ECL with threading support on their end disabled. I would be surprised if, with that configuration, it is still possible to have multiple threads configured to be able to execute ECL (ECL cares a lot about knowing which threads might be executing ECL code, because they need to be stopped during critical GC events. I expect that all of that is turned off when threading support is turned off). Given that IPython apparently runs tab completion in a separate thread, I think the most straightforward way of solving the immediate problem here is to avoid that ecl code will be run upon tab completion. That can be done by building the completion cache upon build time, rather than on-demand. |
comment:16
Replying to @nbruin:
Right, I think I finally understand your point about signals---sorry for being thick. It's even worse, I think - apart from signals, ecllib does non-thread-safe things to global variables...
That is we potentially might still get hit by many threads here, even if something seemingly innocent happens. To me it looks that to disable threads in tab completion is a more robust solution, and it will also make sure that other extensions are safe and sound in this respect, not only ECL/Maxima. |
comment:17
Replying to @dimpase:
After initialization that should pretty much be limited to the modifications that are made to the ECL doubly linked list maximalib is a different issue: maxima is just not thread-safe in its design at all. So I don't think it's worth investing in making ecllib thread-safe (and the signals are a real obstruction), because our main application doesn't allow it anyway. |
comment:18
Replying to @nbruin:
It seems that It is true that signals and threads generally do not mix well. Signal handlers are set on the level of the process, not threads. |
This comment has been minimized.
This comment has been minimized.
comment:20
I don't think that signal handling has anything to do with this bug here. I don't see any signals being raised in a |
comment:21
Wait a minute... the "stack overflow" reminds me of a very similar issue that affected PARI/GP: #17773 |
comment:22
Boehm GC (which is used by ECL) installs its own signal handlers in order to be able to scan for garbage. Thus if another thread does something to signals, then GC and thus ECL might go belly up. "Stack overflow" might be due to GC being given data to work on from another thread it does not know about. The more I think about it the more inclined I become towards disabling tab completion in a separate thread. |
comment:23
Replying to @dimpase:
I understand what you are saying but I don't believe that this has anything to do with this ticket. |
comment:24
Replying to @jdemeyer:
As Nils explains, Maxima is not thread-safe, and thus invoking it from non-main thread (e.g. from the tab-completion one) is prone to errors. Thus invoking ECL from non-main thread does not need to be allowed. |
comment:25
Right. There are two issues:
This ticket is about the second issue. It can be fixed independently. |
comment:26
It turns out that both issues are actually relevant. After fixing the second issue (in plain IPython, not Sage):
The
So it seems that running ECL only in the main thread is the only solution. |
comment:27
Replying to @jdemeyer:
ECL does have facilities for registering/de-registering threads, IMHO making this work seems to be a tough call, and in particular in the upcoming ECL 16.2 this code (and the signals-handling code) is being changed, so what works for 16.1.2 might break in the next version. |
comment:28
Replying to @dimpase:
In that case: perhaps downgrade from blocker and solve later? The issue is a serious one, but the symptoms seem to be easily avoided (it's a rather specific tab completion). Concerning threading: at least a while ago, enabling threading in ECL meant that ECL would start up a dedicated signal handling thread, and really start using (very strange!) signals to signal GC events to other threads. That setup looked very hard to make compatible with sage. That's why I think we want to stick with ECL without threading (and hope they keep supporting that! They really should if they want to keep the "embeddable" a serious option, because in many embedding scenarios having the library take control of signal handling in such an invasive way will be very hard to work with.) |
comment:29
I was just pointed out at a solved IPython issue I missed, which reverts reliance on prompt_toolkit, and brings back single-threading behaviour of IPython: There is another reason for avoiding prompt_toolkit - it does multi-threaded importing of Python modules, and given how fragile Sage is in its dependencies handling, this is something to avoid, unless we want more mysterious crashes to happen. |
comment:30
By the way, FriCAS is another (soon to be optional (see #23847), currently experimental) Sage package dependent on ECL in a substantial way. |
comment:31
Replying to @dimpase:
That shouldn't interact with the issue here at all, as long as FriCas runs via a proper expect interface. If people start running FriCas in ecllib, I expect bigger trouble, because I don't expect that one can run maxima and fricas in the same lisp without special measures -- both are legacy applications that were originally designed to have the world (or at least their process) to themselves. Getting rid of multi-threading in IPython sounds like a very good idea. |
comment:32
Here is one way to try the old new IPython prompt---this gets rid of crashes for me.
once at IPython prompt, type
and quit. Then IPython will use readline for completion, as in good old days of version 4.x.
(notice the 1st import---needed to initialise Sage). |
comment:33
Replying to @nbruin:
One thing I've been investigating--the relevance of which I'm not sure--is that we compile libgc with threading support but ECL without. The implications of this are complicated enough that I don't fully understand yet, but it makes me wonder if this can lead to bugs (this is possibly related to #23973). |
comment:34
Replying to @embray:
...
IMHO one takes care of this in
making sure that signals are not handled in a separate thread, no? That's why I think we want to stick with ECL without threading (and hope they keep supporting that! They really should if they want to keep the "embeddable" a serious option, because in many embedding scenarios having the library take control of signal handling in such an invasive way will be very hard to work with.)
Mind you, I came to this ticket via #23956 via #22679; on the latter I had a lot of trouble with threads (yes, in docbuilding with I am not sure what "multithreading for GC" really means; it can be any combination of 2 things:
IMHO 1) is disabled by |
comment:35
I don't think that this should be a blocker issue. It's an annoying bug which crashes Sage, but it's not very likely to appear since the TAB completion is cashed. |
comment:36
I already mentioned in comment 9 above that even with the TAB cache present, one can get an annoying runtime error; to replicate,
and choose something from the list that pops up, then the following input leads to a runtime error.
On the positive side, ipython folks are apparently going to disable tab completion in a separate thread, once a version of |
comment:37
It appears that this has nuked GAP pexpect interface, too:
and Sage becomes irresponsive and has to be killed. |
comment:38
Replying to @dimpase:
This doesn't happen to me with Sage 8.2.beta6, or maybe I don't understand the necessary steps. If I run Sage and then immediately run "gap.", it works fine (OS X 10.13.3). Am I missing some aspect of triggering this? |
comment:39
Confirmed that |
comment:40
Moreover if I |
comment:41
Replying to @rwst:
Sorry, it appears that in case of |
comment:42
all is good in Sage 9.3.rc1 |
Reviewer: Dima Pasechnik |
If Maxima's commands list is not stored, then initialising Maxima/ECL and then hitting TAB after
maxima_lib
crashes Sage, as shown below. Other similar crashes may be triggered, see e.g. #23956.The reason for these crashes is the design of tab completion in
IPython
5+ usingprompt_toolkit, which uses Python threading, and does tab completion in a separate thread.
Depends on #23956
CC: @jdemeyer
Component: interfaces
Reviewer: Dima Pasechnik
Issue created by migration from https://trac.sagemath.org/ticket/22766
The text was updated successfully, but these errors were encountered: