gnucash master: Bug 798778 - GnuCashquits abruptly when attempting to edit options…
John Ralls
jralls at code.gnucash.org
Fri Mar 17 13:42:31 EDT 2023
Updated via https://github.com/Gnucash/gnucash/commit/b4b84319 (commit)
from https://github.com/Gnucash/gnucash/commit/144b6ae0 (commit)
commit b4b843198421aef9332ad1cae348a4acacfa5586
Author: John Ralls <jralls at ceridwen.us>
Date: Thu Mar 16 17:50:06 2023 -0700
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.
diff --git a/bindings/guile/gnc-optiondb.i b/bindings/guile/gnc-optiondb.i
index ce5c9433c8..2343daf49a 100644
--- a/bindings/guile/gnc-optiondb.i
+++ b/bindings/guile/gnc-optiondb.i
@@ -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));
}
diff --git a/gnucash/gnome-utils/dialog-options.cpp b/gnucash/gnome-utils/dialog-options.cpp
index cec82663f1..af93d9822a 100644
--- a/gnucash/gnome-utils/dialog-options.cpp
+++ b/gnucash/gnome-utils/dialog-options.cpp
@@ -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);
diff --git a/libgnucash/engine/gnc-option.hpp b/libgnucash/engine/gnc-option.hpp
index f561414a0d..b2f9ffb8a7 100644
--- a/libgnucash/engine/gnc-option.hpp
+++ b/libgnucash/engine/gnc-option.hpp
@@ -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
Summary of changes:
bindings/guile/gnc-optiondb.i | 24 ++++++++++++++++++++++--
gnucash/gnome-utils/dialog-options.cpp | 30 ++++++++++++++++++++++++++----
libgnucash/engine/gnc-option.hpp | 9 +++++----
3 files changed, 53 insertions(+), 10 deletions(-)
More information about the gnucash-changes
mailing list