freetype.Face.__del__: check first if FT_Face_Done has been set to None (#169)

[carandraug]

When we're called during the interpreter shutdown, the functions we need may already be set to None. So check for that before trying to call them which fixes the "'NoneType' object is not callable" errors described in issues #44 and #169.

[HinTak]

Not quite. FT_Done_Face is lib.FT_Done_Face so does not need to be none to break. lib could be none or lib.FT_Done_Face could be none too, in this idea...

[carandraug]

Not quite. FT_Done_Face is lib.FT_Done_Face so does not need to be none to break. lib could be none or lib.FT_Done_Face could be none too, in this idea...

FT_Done_Face is a reference to lib.FT_Done_Face. Even if lib is set to None, FT_Done_Face should continue working.

Anyway, on my current case (where this bug is triggered almost always), this check seems to be enough.

[carandraug]

I only have one uncertainty about this. The end result is that FT_Done_Face will not be called. But that's ok because we're at the stage that the program is exiting anyway (right?).

[HinTak]

But in the desired outcome, FT_Done_Face (and FT_Done_Library) should be run, without explicit del. To allow multiple faces to used, for example. Otherwise it is no better than do-nothing (and leak face handles) on destruction.

[carandraug]

But in the desired outcome, FT_Done_Face (and FT_Done_Library) should be run, without explicit del. To allow multiple faces to used, for example. Otherwise it is no better than do-nothing (and leak face handles) on destruction.

I think that the desired outcome is to not have memory leaked. This requires calling FT_Done_Face (and similar) which is is only available via Face.__del__. But in the case that the program is about to exit, then it doesn't matter if those functions are not called because the memory will be freed anyway.

There are multiple situations that need to be "covered":

1. During "normal runtime", Face.__del__ is called when a Face object is about to be destroyed. This is the case where calling FT_Done_Face is particularly important to avoid memory leaks. This is working as expected.

2. When the program is about to exit, three different things can happen:

   1. Face.__del__ may be called early during the interpreter shutdown and have all the functions and variables still available to it. In this situation the current code has no issue and everything is working as expected.
   2. Face.__del__ may be called later during interpreter shutdown and some variables may already be set to None. This is the case that this PR fixes. In this case, FT_Done_Face has ben set to None and the function is no longer available to be called. But we're about to exit the program so it doesn't matters whether FT_Done_Face is actually called because the memory is going to be freed as the program exits.

   3. Face.__del__ is not called at all. From Python's documentation itself:

      It is not guaranteed that __del__() methods are called for objects that still exist when the interpreter exits.

      In this case, FT_Done_Face is never called. As in the case above, it doesn't matter because the program is about to exit and the memory be freed anyway.

   I have seem all these three cases while testing this.

---

The final point is, I think, this: if you want to ensure that FT_Done_Face is called when the program exits, you can't rely on __del__ (but I don't think you need to ensure this).

[carandraug]

I'm happy to discuss this further over a video call if you prefer.

[rougier]

Looks simple enough to me. What do you think @HinTak ?

[HinTak]

The change itself is fine. But I think the problem itself is a bit bigger - namely that freetype-py doesn't currently call FT_Done_X for most X. The most interesting case is perhaps something like a loop which read say, a collection of 100 fonts, one after another, and do stuff with it. Those who care about this situation, at the moment, can do an explicit del Face within the loop (and even that probably does not clean up bitmaps, etc).

I'd say merge this, but keep the issue open.

[rougier]

@carandraug Can you open an issue referring to this pull request about the potential problems mentioned by @HinTak?