Skip to content

Commit

Permalink
Fix crash when changing auto-completer in wxGTK
Browse files Browse the repository at this point in the history
Recent changes resulted in crashes when handling grab-notify signal in
an already deleted object.

Fix this by disconnecting our grab-notify handler when destroying the
object, unless the entire associated wxTextEntry is being destroyed (in
which case no such signals risk to be generated anyhow).

In order to be able to do this safely, store the widget to whose signal
we had connected and check that the widget is still valid before
disconnecting. This also allows to simplify the code by getting rid of
DisableCompletion() and just doing the cleanup in dtor.

Closes wxWidgets#18084.
  • Loading branch information
vadz committed Feb 18, 2018
1 parent c83ddec commit 8278f7b
Showing 1 changed file with 24 additions and 23 deletions.
47 changes: 24 additions & 23 deletions src/gtk/textentry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -235,18 +235,19 @@ class wxTextAutoCompleteData
GetEditableWindow(m_entry)->SetWindowStyleFlag(flags);
}

void DisableCompletion()
{
gtk_entry_set_completion (GetGtkEntry(), NULL);

g_signal_handlers_disconnect_by_data(GetGtkEntry(), this);
}

virtual ~wxTextAutoCompleteData()
{
// Do not call DisableCompletion() here, it would result in problems
// when this object is destroyed from wxTextEntry dtor as the real
// control (e.g. wxTextCtrl) is already destroyed by then.
// Note that we must not use m_entry here because this could result in
// using an already half-destroyed wxTextEntry when we're destroyed
// from its dtor (which is executed after wxTextCtrl dtor, which had
// already destroyed the actual entry). So use the stored widget
// instead and only after checking that it is still valid.
if ( GTK_IS_ENTRY(m_widgetEntry) )
{
gtk_entry_set_completion(m_widgetEntry, NULL);

g_signal_handlers_disconnect_by_data(m_widgetEntry, this);
}
}

protected:
Expand All @@ -260,14 +261,15 @@ class wxTextAutoCompleteData

explicit wxTextAutoCompleteData(wxTextEntry* entry)
: m_entry(entry),
m_widgetEntry(entry->GetEntry()),
m_origWinFlags(GetEditableWindow(m_entry)->GetWindowStyleFlag())
{
GtkEntryCompletion* const completion = gtk_entry_completion_new();

gtk_entry_completion_set_text_column (completion, 0);
gtk_entry_set_completion (GetGtkEntry(), completion);
gtk_entry_set_completion(m_widgetEntry, completion);

g_signal_connect (GTK_WIDGET(GetGtkEntry()), "grab-notify",
g_signal_connect (m_widgetEntry, "grab-notify",
G_CALLBACK (wx_gtk_entry_parent_grab_notify),
this);
}
Expand All @@ -280,8 +282,6 @@ class wxTextAutoCompleteData
return entry->GetEditableWindow();
}

GtkEntry* GetGtkEntry() const { return m_entry->GetEntry(); }

// Helper function for appending a string to GtkListStore.
void AppendToStore(GtkListStore* store, const wxString& s)
{
Expand All @@ -293,7 +293,7 @@ class wxTextAutoCompleteData
// Really change the completion model (which may be NULL).
void UseModel(GtkListStore* store)
{
GtkEntryCompletion* const c = gtk_entry_get_completion (GetGtkEntry());
GtkEntryCompletion* const c = gtk_entry_get_completion(m_widgetEntry);
gtk_entry_completion_set_model (c, GTK_TREE_MODEL(store));
gtk_entry_completion_complete (c);
}
Expand All @@ -302,6 +302,9 @@ class wxTextAutoCompleteData
// The text entry we're associated with.
wxTextEntry * const m_entry;

// And its GTK widget.
GtkEntry* const m_widgetEntry;

// The original flags of the associated wxTextEntry.
const long m_origWinFlags;

Expand Down Expand Up @@ -709,14 +712,16 @@ bool wxTextEntry::DoAutoCompleteStrings(const wxArrayString& choices)
// Try to update the existing data first.
if ( !m_autoCompleteData || !m_autoCompleteData->ChangeStrings(choices) )
{
delete m_autoCompleteData;
m_autoCompleteData = NULL;

// If it failed, try creating a new object for fixed completion.
wxTextAutoCompleteFixed* const ac = wxTextAutoCompleteFixed::New(this);
if ( !ac )
return false;

ac->ChangeStrings(choices);

delete m_autoCompleteData;
m_autoCompleteData = ac;
}

Expand All @@ -730,12 +735,6 @@ bool wxTextEntry::DoAutoCompleteCustom(wxTextCompleter *completer)
{
if ( m_autoCompleteData )
{
// This is not done in dtor because it's unnecessary when replacing
// one completer with another one, and even dangerous, when the
// control is being destroyed anyhow, so we need to call it
// explicitly here to really disable completion.
m_autoCompleteData->DisableCompletion();

delete m_autoCompleteData;
m_autoCompleteData = NULL;
}
Expand All @@ -748,14 +747,16 @@ bool wxTextEntry::DoAutoCompleteCustom(wxTextCompleter *completer)
if ( !m_autoCompleteData ||
!m_autoCompleteData->ChangeCompleter(completer) )
{
delete m_autoCompleteData;
m_autoCompleteData = NULL;

wxTextAutoCompleteDynamic* const
ac = wxTextAutoCompleteDynamic::New(this);
if ( !ac )
return false;

ac->ChangeCompleter(completer);

delete m_autoCompleteData;
m_autoCompleteData = ac;
}
}
Expand Down

0 comments on commit 8278f7b

Please sign in to comment.