Loading the PIL.Image module twice during the same process causes AccessInit: hash collision #2268

Open
tribetiki opened this Issue Dec 1, 2016 · 5 comments

Projects

None yet

4 participants

@tribetiki

What did you do?

We have an embedded c++ Python program that runs scripts in a clean Python environment by always creating a fresh environment with Py_InitializeEx and Py_Finalize. I tried adding some image processing with PIL.Image but that causes the application to crash on the second run of the script. I traced this down to the import throwing an AccessInit: hash collision exception.

What did you expect to happen?

The module should load properly after the Python state has been cleared and re-created.

What actually happened?

The module failed to load on the second run.

What versions of Pillow and Python are you using?

Pillow 3.4.2
Python 2.7.11 32-bit
Windows 10

Code to reproduce

#include "Python.h"

int main(int argc, char* argv[])
{
	Py_SetPythonHome( "c:\\Python27" );

	Py_InitializeEx( 0 );
	Py_DECREF(PyImport_ImportModule( "PIL.Image" ));
	Py_Finalize();

	Py_InitializeEx( 0 );
	Py_DECREF(PyImport_ImportModule( "PIL.Image" ));
	Py_Finalize();

	return 0;
}

Output:

AccessInit: hash collision: 22 for both 1 and 1
@wiredfool
Member

You're beyond what I understand. I tried to build this, but couldn't get it to link (from vs2015) so I wasn't able to investigate it.

From reading the docs, it appears that some variables may not be freed and dlls may not be unloaded, so that points to something in there that's hanging around. One approach may be to investigate what memory has been alloc'd but not freed just before the second call to Py_IntitalizeEx. Valgrind or similar may be useful there.

@tribetiki

My apologies. The simplified example was a bit "too simplified". Python's c headers use pragma directives to specify the link library which for the debug build does not exist in vanilla Python installation. I'm attaching the whole project that has all the include and library paths set up (assumes Python is installed to c:\Python27). The Visual Studio solution is for 2012 but I think 2015 can upgrade it automatically when opening.

Attached solution: PillowTest.zip

@tribetiki

I did some digging around the Pillow source code. Well basically I just did a search to see where that error is thrown from: https://github.com/python-pillow/Pillow/blob/6e7553fb0f12025306b2819b9b842adf6b598b2e/libImaging/Access.c#L36

Access.c seems to use a statically allocated access table which won't get destroyed when the Python state is destroyed since the dll itself will stay in memory. I think that somehow the functionality should either be A) fixed to be re-entrant or B) de-initialized properly when Python state is destroyed. I think going for option A) might be a lot simpler. But then again I haven't really investigated this further so I'll leave that up to you guys!

Cheers!

@tribetiki
tribetiki commented Dec 7, 2016 edited

Upon further investigation I believe this could be fixed by simply changing line 35 in add_item like this:

if (strcmp(access_table[i].mode, mode) != 0) {

This way the error would be thrown from an actual hash conflict, not when trying to re-set the same mode again with the same hash key.

@cgohlke
Contributor
cgohlke commented Dec 7, 2016

This will do for testing:

import sys
import subprocess
from distutils import ccompiler

with open('embed_pil.c', 'w') as fh:
    fh.write("""
#include "Python.h"

int main(int argc, char* argv[])
{
    Py_SetPythonHome( "%s" );

    Py_InitializeEx( 0 );
    Py_DECREF(PyImport_ImportModule( "PIL.Image" ));
    Py_Finalize();

    Py_InitializeEx( 0 );
    Py_DECREF(PyImport_ImportModule( "PIL.Image" ));
    Py_Finalize();

    return 0;
}    
    """ % sys.prefix.replace('\\', '\\\\'))

compiler = ccompiler.new_compiler()
objects = compiler.compile(['embed_pil.c'])
compiler.link_executable(objects, 'embed_pil')

subprocess.call(['embed_pil.exe'])
@wiredfool wiredfool added a commit to wiredfool/Pillow that referenced this issue Dec 20, 2016
@wiredfool wiredfool Fix Access to be reloaded if the python interpreter is restarted when…
… embedded. Fixes #2268.
a7f0b3a
@aclark4life aclark4life added this to the 4.1.0 milestone Jan 8, 2017
@aclark4life aclark4life added the Bug label Jan 8, 2017
@wiredfool wiredfool added a commit to wiredfool/Pillow that referenced this issue Jan 11, 2017
@wiredfool @wiredfool wiredfool + wiredfool Fix Access to be reloaded if the python interpreter is restarted when…
… embedded. Fixes #2268.
9a0dfec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment