rougier / freetype-py

Python binding for the freetype library
Other
303 stars 88 forks source link

Segmentation fault when loading multiple fonts #56

Open rspeer opened 7 years ago

rspeer commented 7 years ago

I can reliably make freetype-py segfault. This may or may not be an instance of #44, but I have a rather minimal script that causes the segfault:

import freetype

FONTS = [
    '/usr/share/fonts/truetype/noto/NotoSans-Regular.ttf',
    '/usr/share/fonts/opentype/noto/NotoSansCJK-Regular.ttc',
]

def arbitrary_function(x):
    pass

for filename in FONTS:
    face = freetype.Face(filename)

I tried removing the unused function, but the segfault stopped happening when I did.

Information about my system:

HinTak commented 7 years ago

Try adding a del face at the end of the loop.

I have found that the garbage collector in python is not very clever - it will free the library handle first before freeing the face handle, thus causing segfault when a script ends (usually not important).

rspeer commented 7 years ago

Ah, I understand the problem, and thanks for the quick response.

Could you perhaps check in the destructor whether the library handle is gone, and bail out of the destructor if so? The program is about to exit, and whether destructors run at the program exit is usually not important and also not guaranteed by Python anyway.

Otherwise I think you should at least document the destructor issue, saying that any global variables referring to a Face need to be deled before the program exits.

I know I could ignore that my program exits by segfaulting, but that is hard to ignore when using freetype in a script that's part of a larger build process.

HinTak commented 7 years ago

It is fairly common knowledge among people who use freetype in C: the library handle should lives longer than the face handle which in turn lives longer than the glyph handle. Quite logically really. When you expect these things to go away automatically in the right order in a garbage-collecting language, that's a different matter.

You might be able to "fix" the issue by obtaining the library handle outside the loop - that may hint at python to keep it longer I.e. add a line library = get_handle() near the beginning.

rspeer commented 7 years ago

I'm sure this is all very logical in C, but you're writing a Python package, and the fact that you expect users to do a bit of C-like lifetime management isn't documented.

The __del__ methods seem to be doing generally the right thing, making things go away automatically in the right order, as you say. The exception is at program exit. What would be wrong with bailing out of __del__ at program exit?

anthrotype commented 7 years ago

Registering an atexit handler might do the trick. E.g. see https://github.com/ldo/harfpy/commit/8a12691feb52a7308465d540aea7b2f3575072cf

HinTak commented 7 years ago

@anthrotype thanks for the tips on atexit . That sounds like a good direction. Though I wouldn't follow the harfpy approach in detail. Modifying classes on the fly sounds like asking for trouble. I don't like the idea that classes in memory differing from class codes in source files. Also a somewhat personal grip - ldo's coding style is rather unusual and painful to read at times...

It is a pity the library handle has no knowledge of opened fonts, etc (AFAIK, I could be wrong). I think it recursively cleaning up on exit would be more logical.

rspeer commented 7 years ago

I think you're making this more complicated than it needs to be. There is no need for memory management when the program is exiting. The memory is going to be freed no matter what you do.

The harfpy code is a pretty concise way to say "look, these methods don't matter anymore, don't run them".

HinTak commented 7 years ago

Actually if you don't care about memory management, a even simpler approach is to not have any destructors, and let it leak :-).

I think code in memory differing from source code in files is asking for trouble in the long run.

The c library handle does not track the C face handles, but the python wrappers around these two can track each other. I think that's cleaner.

rspeer commented 7 years ago

I do care about memory management, just not while the process is exiting.

HinTak commented 7 years ago

I think if the python face object knows about the library object (as the C handle does) and avoid calling the c destructor if the library object is gone, that could work too.

rougier commented 7 years ago

Can't we register every Face created during creation and make sure they're deleted before the library handle is freed when exiting?

anthrotype commented 7 years ago

yes, that could do as well. The face object could keep a weakref to the parent library object, and if that is not None skip calling the destructor.

HinTak commented 7 years ago

That could do also... but I think just having the face's destructor knowning and checking the library's handle would do. The face's contructor's know about the library's handle (but currently does not keep it).


On Tue, 2/5/17, Nicolas P. Rougier notifications@github.com wrote:

Can't we register every Face created during creation and make sure they're deleted before the library handle is freed when exiting?