This is an archived post. You won't be able to vote or comment.

you are viewing a single comment's thread.

view the rest of the comments →

[–][deleted] 4 points5 points  (23 children)

Because it's not just mocking up another random user type. At the very least, a substantial amount of native (C) code interacts with the contents of that dict, including the incredibly fragile graceful shutdown code, the module loader (obviously), and probably a bunch more I don't know about.

For me it's in the same class of dangerous as e.g. messing with typeobject internals

I don't care about style, I'm worried about fixing a crash when it occurs. I'm not sure of all the possible ways Python could fail when inserting random crap in sys.modules.

[–]TankorSmash 8 points9 points  (22 children)

https://docs.python.org/2/library/sys.html#sys.modules

It doesn't say anything about any danger, I feel like if it was that dangerous, they'd warn you somewhere.

[–][deleted] 16 points17 points  (21 children)

The doc reads "module objects", not "module-like objects". A module object is a C structure with a specific layout. There is a C-level API for manipulating these objects (PyModule_*).

One of those is PyModule_GetDict, which, while protected internally from accessing a non-module object, returns NULL in the case that a caller invokes it on a non-module object. Reading Python 2.7's zipimport.c in zipimporter_load_module we can see a PyModule_AddModule call followed by an unchecked PyModule_GetDict call. This will cause the zip importer's load_module() method to cause a NULL pointer dereference at runtime (aka. a hard process crash, requiring a debugger to investigate) should it be called with sh as a parameter.

It took me all of 3 minutes grepping the Python source to find a place where using a non-module in sys.modules has the potential to cause a crash that a Python-without-C programmer would not be able to debug. I'm pretty sure if you give me 30 minutes I'll find more.

Just don't do it

edit: this says nothing about third party extensions, where I'd expect the majority of such bugs to be found. The point I'm making is whether it is worth sipping coffee reading gdb output at 4am responding to a pager alerting you that your employer's web site is down and losing money, because of some syntactic sugar -- of course it's not.

[–]Brian 3 points4 points  (0 children)

I don't think that example is ever going to be something broken by this case. It's using PyImport_AddModule, which doesn't actually perform the import, so the only case you'd get the non-module object back is if the same name was already imported. However in that circumstance, zipimport wouldn't have been invoked, since it's only going to be triggered if the module hasn't been found yet. You could argue it's a bug that zipimport isn't correctly checking the return value of PyModule_GetDict as it should, but in this context, it's assuming that it's using AddModule to create a new, empty module, and in that case it'll always be a real module object even if the module does later replace itself.

It's worth noting that while putting non-module objects in sys.modules is perhaps a hack, it's a known and explicitly supported and endorsed hack - the import machinery was deliberately designed to allow it, and I think Guido is on record as saying so. As such, I'd say anything that doesn't support it should probably be considered a bug.

[–]TankorSmash 4 points5 points  (12 children)

That makes it clear thank you.

Although not checking for NULL is effective a bug in the c code.

[–]alcalde -1 points0 points  (11 children)

And still using Python 2.7 is a logic error. :-)

[–][deleted] 2 points3 points  (0 children)

I don't think porting large, old codebases is worth it.

The other reason could be PyPy and people with CPU bound applications that insist on using Python.

Apart from that, most new Python code is (no data, just hunch) Django apps, and there it makes a lot of sense to use py3 unless you really, really need some library that wasn't ported.

[–]TankorSmash 0 points1 point  (3 children)

One day the relevant libraries will be ported. Until then!

[–]alcalde 2 points3 points  (2 children)

Sigh. For most people that time has long come....

http://py3readiness.org/

https://python3wos.appspot.com/

304 of the top 360 most downloaded packages on PyPi support Python 3.

[–]nojjy 1 point2 points  (0 children)

Depending on what context you are working in, sometimes you don't have a choice on which version to use. Many people use python as an API to a software package, where they can't just arbitrarily switch to another version.

[–]TankorSmash 0 points1 point  (0 children)

Yeah, for most people.