FIX: add check if the renderer exists #5060
Conversation
| # be called to create the renderer and to render the initial view | ||
| # of the figure | ||
| if not hasattr(self, 'renderer'): | ||
| FigureCanvasAgg.draw(self) |
There was a problem hiding this comment.
This is probably unsafe. paintEvent is usually called by a different thread, not from main thread the matplotlib code is running in. Calling draw here may cause concurrent access to the matplotlib figure, which is not thread safe.
There was a problem hiding this comment.
Change to return as you suggested.
|
Works for me. |
There exists a path where paintEvent can be called before the first time that `draw` has been called. This results in an exception because the call to `FigureCanvasAgg.draw(self)` is what create the renderer attribute on the canvas.
83150d7 to
16baab9
Compare
|
Works for me too 👍 |
|
I will self merge this later if no one beats me to it. |
FIX: add check if the renderer exists
|
Couldn't resist the rare opportunity to beat you to it! I had also given a quick check to the revised version. |
|
I don't think there is much to clean up. You can't control (and shouldn't) when paintEvent is called, so it's perfectly possible that paintEvent is issued while matplotlib hasn't done anything yet. So there is just the question of using |
|
In principle I agree with you. However, I was seeing this (I think) consistently on OSX, so I'm not positive it's race condition rather than poor ordering. |
|
On the other hand I never saw this on linux which makes me worried that if it is ordering, not a race condition, it is at a layer we can not easily control. |
|
Well, I'm sure that there is a certain kind of order and consistent behaviour on each specific platform. But you could call it a non-random race condition because you don't know and cannot rely on the way Qt is implemented on a system. It might as well change on the next release. I think not making assumptions regarding the timing of paintEvent is the safest path. |
There exists a path where paintEvent can be called before the first time
that
drawhas been called. This results in an exception because thecall to
FigureCanvasAgg.draw(self)is what create the rendererattribute on the canvas.
Closes #5058
@astrofrog @efiring @pwuertz