Bug 798778 - GnuCashquits abruptly when attempting to edit options…

for certain reports.

Those reports being ones using complex options, apparently because
the callbacks weren't protected from Guile's garbage collector.

So replace the anyway ugly hack of a void* with a std::any wrapping
a class holding a std::unique_ptr with a custom deleter. The
constructor calls scm_gc_protect_object on the SCM containing the
callback and the custom deleter calls scm_gc_unprotect_object. The
copy constructor, required for std::any, makes a new std::unique_ptr
and calls scm_gc_protect_object again ensuring that the protect and
unprotect calls are symmetrical.

Meanwhile std::any hides the Guile dependency from all the classes
that don't need to know about it. The only ugliness is that there's
no good place to put a common implementation of SCNCallbackWrapper so it's
repeated in gnc-optiondb.i and dialog-options.cpp.
This commit is contained in:
John Ralls 2023-03-16 17:50:06 -07:00
parent 144b6ae090
commit b4b8431984
3 changed files with 53 additions and 10 deletions

View File

@ -1614,7 +1614,27 @@ inline SCM return_scm_value(ValueType value)
};
%ignore SCMDeleter;
%ignore SCMCalbackWrapper;
%inline %{
struct SCMDeleter
{
void operator()(SCM cb) {
scm_gc_unprotect_object(cb);
}
};
class SCMCallbackWrapper
{
std::unique_ptr<scm_unused_struct, SCMDeleter> m_callback;
public:
SCMCallbackWrapper(SCM cb) : m_callback{scm_gc_protect_object(cb)} {}
SCMCallbackWrapper(const SCMCallbackWrapper& cbw) : m_callback{scm_gc_protect_object(cbw.get())} {}
SCM get() const { return m_callback.get(); }
};
/**
* Create a new complex boolean option and register it in the options database.
*
@ -1633,7 +1653,7 @@ void gnc_register_complex_boolean_option(GncOptionDBPtr& db,
{
GncOption option{section, name, key, doc_string, value,
GncOptionUIType::BOOLEAN};
option.set_widget_changed (widget_changed_cb);
option.set_widget_changed(std::make_any<SCMCallbackWrapper>(widget_changed_cb));
db->register_option(section, std::move(option));
}
@ -1666,7 +1686,7 @@ gnc_register_multichoice_callback_option(GncOptionDBPtr& db,
std::get<0>(choices.at(0)));
GncOption option{GncOptionMultichoiceValue{section, name, key, doc_string,
defval.c_str(), std::move(choices)}};
option.set_widget_changed(widget_changed_cb);
option.set_widget_changed(std::make_any<SCMCallbackWrapper>(widget_changed_cb));
db->register_option(section, std::move(option));
}

View File

@ -42,6 +42,7 @@
#include "gnc-session.h" // for gnc_get_current_session
#include "gnc-ui.h" // for DF_MANUAL
#include <any>
#include <iostream>
#include <sstream>
@ -119,18 +120,39 @@ GncOptionsDialog::changed() noexcept
set_sensitive(true);
}
struct SCMDeleter {
void operator()(SCM cb) { scm_gc_unprotect_object(cb); }
};
class SCMCallbackWrapper
{
std::unique_ptr<scm_unused_struct, SCMDeleter> m_callback;
public:
SCMCallbackWrapper(SCM cb) : m_callback{scm_gc_protect_object(cb)} {}
SCMCallbackWrapper(const SCMCallbackWrapper& cbw) : m_callback{scm_gc_protect_object(cbw.get())} {}
SCM get() const { return m_callback.get(); }
};
void
gnc_option_changed_widget_cb(GtkWidget *widget, GncOption* option)
{
if (!option || option->is_internal()) return;
auto ui_item{option->get_ui_item()};
g_return_if_fail(ui_item);
auto widget_changed_cb{option->get_widget_changed()};
auto& widget_changed_cb{option->get_widget_changed()};
auto gtk_ui_item{dynamic_cast<GncOptionGtkUIItem*>(ui_item)};
if (widget_changed_cb && gtk_ui_item)
if (widget_changed_cb.has_value() && gtk_ui_item)
{
SCM widget_value{gtk_ui_item->get_widget_scm_value(*option)};
scm_call_1(static_cast<SCM>(widget_changed_cb), widget_value);
try
{
auto cb{std::any_cast<SCMCallbackWrapper>(widget_changed_cb)};
SCM widget_value{gtk_ui_item->get_widget_scm_value(*option)};
scm_call_1(cb.get(), widget_value);
}
catch(std::bad_any_cast& err)
{
PERR("Bad widget changed callback type %s", err.what());
}
}
const_cast<GncOptionUIItem*>(ui_item)->set_dirty(true);
dialog_changed_internal(widget, true);

View File

@ -34,9 +34,11 @@
#define GNC_OPTION_HPP_
#include <glib.h>
#include <any>
#include <string>
#include <iostream>
#include <iomanip>
#include <type_traits>
#include <variant>
#include <memory>
#include <tuple>
@ -199,14 +201,13 @@ public:
*/
std::istream& in_stream(std::istream& iss);
friend GncOptionVariant& swig_get_option(GncOption*);
void set_widget_changed (void* cb) { m_widget_changed = cb; }
void* get_widget_changed () { return m_widget_changed; }
void set_widget_changed (std::any cb) { m_widget_changed = cb; }
std::any& get_widget_changed () { return m_widget_changed; }
private:
inline static const std::string c_empty_string{""};
GncOptionVariantPtr m_option;
GncOptionUIItemPtr m_ui_item{nullptr};
/* This is a hack to reserve space for a Guile callback pointer. */
void* m_widget_changed{};
std::any m_widget_changed{};
};
inline bool