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.

Sign up for GitHub

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

Jump to bottom

Fix issue #2543 segfault in font dealloc after reinit #2548

Merged
merged 4 commits into from Oct 23, 2021
Merged

Fix issue #2543 segfault in font dealloc after reinit #2548

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3105f5d
Fix segfault in font dealloc after reinit
REW1L Apr 3, 2021
ebaf47d
uint -> unsigned int
REW1L Apr 3, 2021
0888cb1
Ensure that font is NULL after freeing resources
REW1L Apr 3, 2021
d495000
Merge branch 'main' into fix-issue-2543
illume Oct 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 11 additions & 2 deletions src_c/font.c
View file
Open in desktop
Expand Up @@ -65,6 +65,7 @@ PyFont_New(TTF_Font *);
#define PyFont_Check(x) ((x)->ob_type == &PyFont_Type)

static int font_initialized = 0;
static uint current_ttf_generation = 0;
static const char font_defaultname[] = "freesansbold.ttf";
static const char pkgdatamodule_name[] = "pygame.pkgdata";
static const char resourcefunc_name[] = "getResource";
Expand Down Expand Up @@ -188,6 +189,7 @@ font_autoquit(void)
if (font_initialized) {
font_initialized = 0;
TTF_Quit();
current_ttf_generation++;
}
}

Expand Down Expand Up @@ -762,9 +764,15 @@ static void
font_dealloc(PyFontObject *self)
{
TTF_Font *font = PyFont_AsFont(self);

if (font && font_initialized)
if (font && font_initialized) {
if (self->ttf_init_generation != current_ttf_generation) {
// Since TTF_Font is a private structure
// it's impossible to access face field in a common way.
int** face_pp = font;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this is enough to stop the problem instead?

TTF_CloseFont(font);
font=NULL;

Copy link
Contributor Author

@REW1L REW1L Apr 3, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of tp_free further we are able not to aware about font. Therefore setting font to NULL is just redundant. Also font_dealloc is called only once after TTF_Quit to get a segfault inside a function TTF_CloseFont.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks for the explanation :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am still not so clear about what

int **face_pp = font;
*face_pp = NULL;

does, exactly. Can you please explain this bit?
Anyways, I guess it is some weird hack, and I see it works because the tests pass :)

*face_pp = NULL;
}
TTF_CloseFont(font);
}

if (self->weakreflist)
PyObject_ClearWeakRefs((PyObject *)self);
Expand Down Expand Up @@ -931,6 +939,7 @@ font_init(PyFontObject *self, PyObject *args, PyObject *kwds)
Py_XDECREF(oencoded);
Py_DECREF(obj);
self->font = font;
self->ttf_init_generation = current_ttf_generation;
return 0;

error:
Expand Down
1 change: 1 addition & 0 deletions src_c/include/pygame_font.h
View file
Open in desktop
Expand Up @@ -29,6 +29,7 @@ typedef struct {
PyObject_HEAD
TTF_Font* font;
PyObject* weakreflist;
uint ttf_init_generation;
} PyFontObject;
#define PyFont_AsFont(x) (((PyFontObject*)x)->font)

Expand Down
10 changes: 10 additions & 0 deletions test/font_test.py
View file
Open in desktop
Expand Up @@ -206,6 +206,16 @@ def test_issue_font_alphablit(self):

self.assertEqual(pre_blit_corner_pixel, post_blit_corner_pixel)

def test_segfault_after_reinit(self):
""" Reinitialization of font module should not cause
segmentation fault """
import gc
font = pygame_font.Font(None, 20)
pygame_font.quit()
pygame_font.init()
del font
gc.collect()

def test_quit(self):
pygame_font.quit()

Expand Down