Try to avoid crashes on long output into Python console in a short time #2253
Conversation
|
@michalgregor, thanks a lot for doing the PR. I'd like to say two things about it:
|
|
The first is easy to provide: I wouldn't know about the second thing. However – for what it's worth – I have been running PyQt 4 applications with Spyder since the patch and haven't noticed any difference in behaviour. |
| @@ -291,7 +291,6 @@ def get_stderr(self): | |||
|
|
|||
| def write_output(self): | |||
| self.shell.write(self.get_stdout(), flush=True) | |||
| QApplication.processEvents() | |||
There was a problem hiding this comment.
Instead of erasing this line, could you comment it like this?
# Commenting the line below fixes Issue 2251
# QApplication.processEvents()
If other problems appear because of this change, it'd be easier to come back to this place later :-)
|
I confirm the crash using your example, but commenting the line doesn't fix the problem for me. Fore reference, here is the console output: |
|
Yes, you are right. I have applied the fix to the 10 PCs I use in my class and I can confirm that we still sometimes experience crashes – although not as consistently as we did with the line uncommented. So, unfortunately, this does not fully fix the issue. The problem must be somewhere else. |
|
I think that by removing the line spyder does less things between each printed line, so the overflow is less likely to occur. As a temporary workaround, you can execute the script in an external terminal instead of the integrated one (there is an option in the script configuration, F6 by default). I don't know if this option is enabled in all platforms. Are you sure you need to print that much lines in succession ? This bug apart, it looks like a bad idea, writing to a file may be more appropriate. |
|
@michalgregor, why don't you use the IPython console instead? It has much better support for these kind of things |
|
I don't like it mainly because it keeps displaying images inline and in a non-interactive manner. I guess that can be configured somehow but since I see no significant downsides – apart from the bug – in using the standard console I don't see why we should switch. Also – yes, we could execute the script in an external terminal, but then we lose most of the integration like the interactive mode and keeping track of variables. I don't see what use there would be in using Spyder at all in that case... In any case – this is supposed to work. Let's focus on fixing it. I know how to work around the problem in the meantime so don't worry about that too much. |
|
Yes, absolutely. This is a definitely a bug that we should fix, but in the meantime at least there is an alternative you can use. In fact, I think this bug is related to issue #1728, and it's not easy to fix. |
|
I'm going to merge this one because according to @michalgregor, it helps to improve things a little bit. |
Try to avoid crashes on long output into Python console in a short time
|
I believe I can provide further input on this interesting issue. I have made some experiments with mutexes (QMutex from QtCore) trying to check for potential thread collisions that could cause the crashes. I am still using the long output example for testing. When I protect ExternalShellBase's write_output method with a mutex, the entire spyder interface locks up before even the first line of the example is printed. If the mutex is made recursive, the interface keeps going and spyder crashes after a while – as is its usual habit on this example. If I instead use tryLock and return from the function if the mutex is locked already the crashes are resolved (obviously at the cost of scrapping some of the output messages). In any case it seems that the issue is indeed thread related and that write_output is being triggered while another one is still in process – this could easily cause a crash if the underlying text insertion operations in Qt's widgets are not thread safe. Since there is no change in behaviour when a recursive mutex is used, it seems that the issue is caused by write_output being called recursively – I guess this could happen if anything is written into stdout while write_output is still in progress. I hope this information helps in resolving the issue. |
|
Guys, I have implemented a simple workaround for this. It uses two mutexes – the first one is used to check whether a different call to write_output is being processed at the moment. If so, the message is stored in a buffer instead of being printed (the other mutex protects the buffer). The buffer is then itself printed before write_output exists. In code it looks something like this: I admit it is not most elegant of solutions, but it does seem to work. Where should I commit and post this so that it can get tested by you and merged? |
|
@michalgregor awesome work :-), thanks! Could you open a new PR in a separate branch? |
|
Yes, please open a pull request against our master branch so we can test it and merge it :-) |
Fixing crashes on very long output being written to the console in a short time.
Fixes #2251