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.

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

Fix unref problems with floating references #2359

Merged
merged 1 commit into from
Jan 25, 2022

Conversation

ermshiperete
Copy link

While trying to get a test client to work, I encountered some problems that this PR fixes. My test client uses ibusimcontext.c from the GTK client and the problems showed when I ran it with debug-enabled GLIB libraries.

When running with debug-enabled GLIB there are several critical errors output: "A floating object ... was finalized. This means
that someone called g_object_unref() on an object that had only a floating reference; the initial floating reference is not
owned by anyone and must be removed with g_object_ref_sink()."

This change fixes this by calling g_object_ref_sink() before g_object_unref() if we have a floating reference.

It also fixes another related problem where we called g_object_unref() on a static IBusText string (created with
ibus_text_new_from_static_string()) for which the API documentation says not to free.

Copy link
Member

@fujiwarat fujiwarat left a comment

Choose a reason for hiding this comment

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

Thank you very much for the patch.
If you wish your patch with the Git sign, please rebase the patch to the latest Git master updating my reviews.
Also please append a line "BUG=#2359" to your Git description .

Otherwise I can modify your patch.

src/ibusinputcontext.c Outdated Show resolved Hide resolved
src/ibusinputcontext.c Outdated Show resolved Hide resolved
src/ibusinputcontext.c Outdated Show resolved Hide resolved
src/ibusinputcontext.c Outdated Show resolved Hide resolved
src/ibusinputcontext.c Outdated Show resolved Hide resolved
src/ibusinputcontext.c Outdated Show resolved Hide resolved
src/ibusinputcontext.c Outdated Show resolved Hide resolved
ermshiperete added a commit to ermshiperete/ibus that referenced this pull request Jan 21, 2022
When running with debug-enabled GLIB there are several critical
errors output: "A floating object ... was finalized. This means
that someone called g_object_unref() on an object that had only
a floating reference; the initial floating reference is not
owned by anyone and must be removed with g_object_ref_sink()."

This change fixes this by calling `g_object_ref_sink()` before
`g_object_unref()` if we have a floating reference.

It also fixes another related problem where we called
`g_object_unref()` on a static IBusText string (created with
`ibus_text_new_from_static_string()`) for which the API documentation
says not to free.

Bug=ibus#2359
@ermshiperete
Copy link
Author

I rebased and updated my PR.

Copy link
Member

@fujiwarat fujiwarat left a comment

Choose a reason for hiding this comment

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

Please put the capital word 'BUG' and full URL to follow other commits.

s|Bug=#2359|BUG=https://github.com/ibus/ibus/pull/2359|

src/ibusinputcontext.c Outdated Show resolved Hide resolved
When running with debug-enabled GLIB there are several critical
errors output: "A floating object ... was finalized. This means
that someone called g_object_unref() on an object that had only
a floating reference; the initial floating reference is not
owned by anyone and must be removed with g_object_ref_sink()."

This change fixes this by calling `g_object_ref_sink()` before
`g_object_unref()` if we have a floating reference.

It also fixes another related problem where we called
`g_object_unref()` on a static IBusText string (created with
`ibus_text_new_from_static_string()`) for which the API documentation
says not to free.

BUG=ibus#2359
@ermshiperete
Copy link
Author

Please put the capital word 'BUG' and full URL to follow other commits.

s|Bug=#2359|BUG=https://github.com/ibus/ibus/pull/2359|

Ahh, sorry, I missed the fact that it's the full URL because I only looked at the commit message in GitHub 😄

@fujiwarat fujiwarat merged commit 5a455b1 into ibus:master Jan 25, 2022
@fujiwarat fujiwarat added this to the 1.5.26 milestone Jan 25, 2022
@ermshiperete ermshiperete deleted the references branch January 27, 2022 14:18
@bdaase
Copy link

bdaase commented Mar 7, 2022

In case anyone comes a long here as well, this is supposed to fix https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/5147

@hadess
Copy link

hadess commented Mar 29, 2022

In case anyone comes a long here as well, this is supposed to fix https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/5147

It is? Seems like it would cause those problems rather than fix them.

@bdaase
Copy link

bdaase commented Mar 29, 2022

It is? Seems like it would cause those problems rather than fix them.

This was expected to be the case in https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/5147#note_1398671 but turned out to not be the case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants