Skip to content
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

[Bug]: matplotlib crashes if _tkinter doesn't have __file__ #23074

Closed
konstin opened this issue May 19, 2022 · 8 comments
Closed

[Bug]: matplotlib crashes if _tkinter doesn't have __file__ #23074

konstin opened this issue May 19, 2022 · 8 comments
Labels
Milestone

Comments

@konstin
Copy link

@konstin konstin commented May 19, 2022

Bug summary

In the python-build-standalone cpython distributions, _tkinter is doesn't have a separate shared library and therefore also doesn't have a __file__ attribute, causing a crash in matplotlib.

Code for reproduction

Tested on ubuntu 20.04


wget https://github.com/indygreg/python-build-standalone/releases/download/20220502/cpython-3.10.4+20220502-x86_64_v3-unknown-linux-gnu-pgo+lto-full.tar.zst
tar xf cpython-3.10.4+20220502-x86_64_v3-unknown-linux-gnu-pgo+lto-full.tar.zst
python/install/bin/python3 -m venv venv
venv/bin/pip install matplotlib
venv/bin/python -c "import matplotlib.pyplot as plt; plt.plot(list(range(10)), list(range(10)))"

Actual outcome

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/konsti/monotrail/pbsi/venv/lib/python3.10/site-packages/matplotlib/pyplot.py", line 2769, in plot
    return gca().plot(
  File "/home/konsti/monotrail/pbsi/venv/lib/python3.10/site-packages/matplotlib/pyplot.py", line 2274, in gca
    return gcf().gca(**kwargs)
  File "/home/konsti/monotrail/pbsi/venv/lib/python3.10/site-packages/matplotlib/pyplot.py", line 867, in gcf
    return figure()
  File "/home/konsti/monotrail/pbsi/venv/lib/python3.10/site-packages/matplotlib/pyplot.py", line 808, in figure
    manager = new_figure_manager(
  File "/home/konsti/monotrail/pbsi/venv/lib/python3.10/site-packages/matplotlib/pyplot.py", line 326, in new_figure_manager
    _warn_if_gui_out_of_main_thread()
  File "/home/konsti/monotrail/pbsi/venv/lib/python3.10/site-packages/matplotlib/pyplot.py", line 316, in _warn_if_gui_out_of_main_thread
    if (_get_required_interactive_framework(_get_backend_mod())
  File "/home/konsti/monotrail/pbsi/venv/lib/python3.10/site-packages/matplotlib/pyplot.py", line 217, in _get_backend_mod
    switch_backend(dict.__getitem__(rcParams, "backend"))
  File "/home/konsti/monotrail/pbsi/venv/lib/python3.10/site-packages/matplotlib/pyplot.py", line 268, in switch_backend
    switch_backend(candidate)
  File "/home/konsti/monotrail/pbsi/venv/lib/python3.10/site-packages/matplotlib/pyplot.py", line 288, in switch_backend
    class backend_mod(matplotlib.backend_bases._Backend):
  File "/home/konsti/monotrail/pbsi/venv/lib/python3.10/site-packages/matplotlib/pyplot.py", line 289, in backend_mod
    locals().update(vars(importlib.import_module(backend_name)))
  File "/home/konsti/monotrail/pbsi/python/install/lib/python3.10/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1050, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1027, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1006, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 688, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 883, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "/home/konsti/monotrail/pbsi/venv/lib/python3.10/site-packages/matplotlib/backends/backend_tkagg.py", line 1, in <module>
    from . import _backend_tk
  File "/home/konsti/monotrail/pbsi/venv/lib/python3.10/site-packages/matplotlib/backends/_backend_tk.py", line 24, in <module>
    from . import _tkagg
AttributeError: module '_tkinter' has no attribute '__file__'. Did you mean: '__name__'?

Expected outcome

matplotlib works even if _tkinter is not a shared library, maybe with reduced functionality but it shouldn't crash

Additional information

The code that causes this is

matplotlib/src/_tkagg.cpp

Lines 310 to 322 in f25c2d0

// Handle PyPy first, as that import will correctly fail on CPython.
module = PyImport_ImportModule("_tkinter.tklib_cffi"); // PyPy
if (!module) {
PyErr_Clear();
module = PyImport_ImportModule("_tkinter"); // CPython
}
if (!(module &&
(py_path = PyObject_GetAttrString(module, "__file__")) &&
(py_path_b = PyUnicode_EncodeFSDefault(py_path)) &&
(path = PyBytes_AsString(py_path_b)))) {
goto exit;
}
tkinter_lib = dlopen(path, RTLD_LAZY);

I've previously reported this at indygreg/python-build-standalone#129 where @indygreg noted that "I'll likely need to converse with a matplotlib developer on how to best support this. I would like to have a compatibility story here, as matplotlib is a popular package and I'd like it to be supported.".

CC @indygreg

Operating system

ubuntu 20.04

Matplotlib Version

3.5.2

Matplotlib Backend

n/a

Python version

cpython-3.10.4+20220502-x86_64_v3-unknown-linux-gnu-pgo+lto-full

Jupyter version

n/a

Installation

pip

@QuLogic
Copy link
Member

@QuLogic QuLogic commented May 19, 2022

@QuLogic QuLogic added the GUI/tk label May 19, 2022
@anntzer
Copy link
Contributor

@anntzer anntzer commented May 19, 2022

I don't really have a solution available? If the tkinter symbols are not public, I don't know what we can do...

@tacaswell
Copy link
Member

@tacaswell tacaswell commented May 19, 2022

This is part of our logic to find at runtime the tk libraries.

At a minimum we should not fail in this way (I think just protect it with PyObject_HasAttrString)

I'm not sure how much special casing we would take on to support @indygreg , but will definitely consider proposals !

@anntzer
Copy link
Contributor

@anntzer anntzer commented May 19, 2022

(fwiw I don't think we should try to catch this specific case with PyObject_HasAttrString -- next time it'll be the import that fails, or some other python distro that instead sets __file__ to None, or whatnot; if you really want to catch this, we should just raise a wrapper exception that wraps whatever failed in the whole load process.)

@indygreg
Copy link

@indygreg indygreg commented May 20, 2022

Thanks for filing this @konstin!

What's the fundamental requirement here? From the source code it seems to be that matplotlib needs a function pointer / address of Tk_FindPhoto, Tk_PhotoPutBlock, and Tcl_SetVar (Windows only).

Is this functionality critical? If matplotlib could run with these symbols as optional, what's the user impact? Is that an option worth considering?

As it stands, teaching python-build-standalone to export these symbols is doable. But the reason the symbols were made private is so they don't collide with an external shared library that some random other Python extension could pull in. See indygreg/python-build-standalone#114 for more.

A hacky compromise here could be to alias these special symbols under a non-standard name so they don't conflict with the official symbols. That requires any C code to be aware of the alternate symbol names. That's a tall ask. But it also might be a fair request since this is the only Python C extension I'm aware of trying to access C symbols from libraries that aren't explicitly in libpython. I think you've just gotten lucky so far that this has managed to largely just work.

@tacaswell
Copy link
Member

@tacaswell tacaswell commented May 20, 2022

If we do not find these symbols, the user does not get the tkagg backend.

we should just raise a wrapper exception that wraps whatever failed in the whole load process.

fair, and then catch that in the fallback logic and try the next one?

@anntzer
Copy link
Contributor

@anntzer anntzer commented May 20, 2022

Yes (so that basically means we need to raise an ImportError, as that's what the fallback machinery recognizes -- now you've conviced me we do need to do such a wrapping).

@tacaswell
Copy link
Member

@tacaswell tacaswell commented May 23, 2022

The point of this run-time lookup is so that we do not have to a build-time dependency on tk.

I think this is closed by #23089 (we will now gracefully skip the tkagg backend in python-build-standalone case).

If you want to support mpl + tkagg @indygreg I think this is something you should handle on your side. As I said above, we will definitely consider reasonable patches if needed.

@tacaswell tacaswell added this to the v3.6.0 milestone May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants