From eccbb9884c01da973511e2a2f6a2c7ca7620bcf5 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sun, 30 Aug 2020 11:35:30 +0200 Subject: [PATCH 1/2] Fixed an internal error happening when a librrary isn't registered and destroyed by the GC --- src/db/db/dbLibrary.h | 1 - src/db/db/gsiDeclDbLibrary.cc | 39 ++++++++++++++------ testdata/ruby/basic_testcore.rb | 1 + testdata/ruby/dbLibrary.rb | 16 +++++++- testdata/vendor/ruby/test/unit/attribute.rb | 2 +- testdata/vendor/ruby/test/unit/autorunner.rb | 8 ++-- 6 files changed, 48 insertions(+), 19 deletions(-) diff --git a/src/db/db/dbLibrary.h b/src/db/db/dbLibrary.h index 0c45b0c5a..21cd70061 100644 --- a/src/db/db/dbLibrary.h +++ b/src/db/db/dbLibrary.h @@ -238,7 +238,6 @@ namespace tl */ template <> struct type_traits : public type_traits { typedef tl::false_tag has_copy_constructor; - typedef tl::false_tag has_public_destructor; }; } diff --git a/src/db/db/gsiDeclDbLibrary.cc b/src/db/db/gsiDeclDbLibrary.cc index 88f205373..0476a5087 100644 --- a/src/db/db/gsiDeclDbLibrary.cc +++ b/src/db/db/gsiDeclDbLibrary.cc @@ -78,18 +78,35 @@ static std::string get_technology (db::Library *lib) } } -static void dummy_destroy (db::Library *) { } +static void destroy_lib (db::Library *lib) +{ + if (db::LibraryManager::instance ().lib_ptr_by_name (lib->get_name ()) == lib) { + // Library is registered -> do not delete + } else { + delete lib; + } +} -Class decl_Library ("db", "Library", - gsi::method_ext ("_destroy", &dummy_destroy, - "@brief An inactive substitute for _destroy (delete the object)\n" - "As libraries need to be kept if cells are using them, library objects must " - "not be deleted. Hence the default '_destroy' implementation must not be called. " - "To keep old code working, this substitute is provided. It just returns without " - "deleting the object.\n" - "\n" - "This method has been introduced in version 0.26.7." - ) + +namespace { + +class LibraryClass + : public Class +{ +public: + LibraryClass (const char *module, const char *name, const gsi::Methods &methods, const char *description) + : Class (module, name, methods, description) + { } + + virtual void destroy (void *p) const + { + db::Library *lib = reinterpret_cast (p); + destroy_lib (lib); + } +}; + +} + +LibraryClass decl_Library ("db", "Library", gsi::constructor ("new", &new_lib, "@brief Creates a new, empty library" ) + diff --git a/testdata/ruby/basic_testcore.rb b/testdata/ruby/basic_testcore.rb index 97e97e794..5eee23a67 100644 --- a/testdata/ruby/basic_testcore.rb +++ b/testdata/ruby/basic_testcore.rb @@ -832,6 +832,7 @@ class Basic_TestClass < TestBase end + # TODO: this class is going to be deprecated class X < Data end class Y < Object diff --git a/testdata/ruby/dbLibrary.rb b/testdata/ruby/dbLibrary.rb index cd73007a5..b09540dc3 100644 --- a/testdata/ruby/dbLibrary.rb +++ b/testdata/ruby/dbLibrary.rb @@ -35,11 +35,19 @@ class DBLibrary_TestClass < TestBase lib.register("RBA-unit-test") assert_equal(lib.name, "RBA-unit-test") - assert_equal(lib.id != 0, true) + lib_id = lib.id + assert_equal(lib_id != 0, true) assert_equal(RBA::Library::library_names.member?("RBA-unit-test"), true) - assert_equal(RBA::Library::library_by_name("RBA-unit-test").id, lib.id) + assert_equal(RBA::Library::library_by_name("RBA-unit-test").id, lib_id) + # destroy should not do anything as libraries are not to be removed through the destructor + lib._destroy + assert_equal(RBA::Library::library_by_name("RBA-unit-test").id, lib_id) + assert_equal(lib.destroyed?, true) + + lib = RBA::Library::library_by_name("RBA-unit-test") + assert_equal(lib.destroyed?, false) lib.delete assert_equal(RBA::Library::library_by_name("RBA-unit-test"), nil) @@ -85,6 +93,10 @@ class DBLibrary_TestClass < TestBase lib.layout.create_cell("X") assert_equal(lib.layout.top_cell.name, "X") + # this will actually destroy the library as it is not registered + lib._destroy + assert_equal(lib.destroyed?, true) + end end diff --git a/testdata/vendor/ruby/test/unit/attribute.rb b/testdata/vendor/ruby/test/unit/attribute.rb index cfc9e8d2d..55fe11a8b 100644 --- a/testdata/vendor/ruby/test/unit/attribute.rb +++ b/testdata/vendor/ruby/test/unit/attribute.rb @@ -136,7 +136,7 @@ module Test end @@attribute_observers = StringifyKeyHash.new - def register_attribute_observer(attribute_name, observer=Proc.new) + def register_attribute_observer(attribute_name, &observer) @@attribute_observers[attribute_name] ||= [] @@attribute_observers[attribute_name] << observer end diff --git a/testdata/vendor/ruby/test/unit/autorunner.rb b/testdata/vendor/ruby/test/unit/autorunner.rb index 624c5995f..6d3a53b8c 100644 --- a/testdata/vendor/ruby/test/unit/autorunner.rb +++ b/testdata/vendor/ruby/test/unit/autorunner.rb @@ -15,7 +15,7 @@ module Test PREPARE_HOOKS = [] class << self - def register_runner(id, runner_builder=Proc.new) + def register_runner(id, &runner_builder) RUNNERS[id] = runner_builder RUNNERS[id.to_s] = runner_builder end @@ -33,7 +33,7 @@ module Test @@default_runner = id end - def register_collector(id, collector_builder=Proc.new) + def register_collector(id, &collector_builder) COLLECTORS[id] = collector_builder COLLECTORS[id.to_s] = collector_builder end @@ -46,11 +46,11 @@ module Test ColorScheme[id] = scheme end - def setup_option(option_builder=Proc.new) + def setup_option(&option_builder) ADDITIONAL_OPTIONS << option_builder end - def prepare(hook=Proc.new) + def prepare(&hook) PREPARE_HOOKS << hook end From a5d675304cb579abcb33b8c2ccc6612422ea3773 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sun, 30 Aug 2020 13:06:50 +0200 Subject: [PATCH 2/2] Fixed an issue with deferred method execution in unit test context. --- src/lay/lay/layApplication.cc | 8 +++++++- src/tl/tl/tlDeferredExecutionQt.cc | 6 ++++-- src/tl/tl/tlDeferredExecutionQt.h | 1 + testdata/ruby/imgObject.rb | 2 +- 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/lay/lay/layApplication.cc b/src/lay/lay/layApplication.cc index 184baa8f7..558e3cf43 100644 --- a/src/lay/lay/layApplication.cc +++ b/src/lay/lay/layApplication.cc @@ -776,7 +776,10 @@ ApplicationBase::init_app () // establish the configuration dispatcher ()->config_setup (); - // Some info output + // deferred method processing for those plugins which need this + process_events (); + + // some info output if (tl::verbosity () >= 20) { tl::info << "KLayout path:"; @@ -1504,6 +1507,9 @@ GuiApplication::process_events_impl (QEventLoop::ProcessEventsFlags flags, bool mp_mw->enter_busy_mode (true); try { QApplication::processEvents (flags); + // Qt seems not to send posted UserEvents in some cases (e.g. in the unit test application with GLib? + // Glib not doing this without a main window visible?). Hence we do this explicitly here. + QApplication::sendPostedEvents (); } catch (...) { // ignore exceptions } diff --git a/src/tl/tl/tlDeferredExecutionQt.cc b/src/tl/tl/tlDeferredExecutionQt.cc index 1f67632a9..6e1c125a5 100644 --- a/src/tl/tl/tlDeferredExecutionQt.cc +++ b/src/tl/tl/tlDeferredExecutionQt.cc @@ -32,6 +32,8 @@ namespace tl DeferredMethodSchedulerQt::DeferredMethodSchedulerQt () : QObject (qApp), DeferredMethodScheduler () { + m_event_type = QEvent::registerEventType (); + connect (&m_timer, SIGNAL (timeout ()), this, SLOT (timer ())); m_timer.setInterval (0); // immediately @@ -51,13 +53,13 @@ DeferredMethodSchedulerQt::~DeferredMethodSchedulerQt () void DeferredMethodSchedulerQt::queue_event () { - qApp->postEvent (this, new QEvent (QEvent::User)); + qApp->postEvent (this, new QEvent (QEvent::Type (m_event_type))); } bool DeferredMethodSchedulerQt::event (QEvent *event) { - if (event->type () == QEvent::User) { + if (event->type () == m_event_type) { timer (); return true; } else { diff --git a/src/tl/tl/tlDeferredExecutionQt.h b/src/tl/tl/tlDeferredExecutionQt.h index f9afba7f6..06f0e2e48 100644 --- a/src/tl/tl/tlDeferredExecutionQt.h +++ b/src/tl/tl/tlDeferredExecutionQt.h @@ -65,6 +65,7 @@ private slots: private: QTimer m_timer, m_fallback_timer; + int m_event_type; virtual bool event (QEvent *event); }; diff --git a/testdata/ruby/imgObject.rb b/testdata/ruby/imgObject.rb index 7c1af0ae8..2a8f9a853 100644 --- a/testdata/ruby/imgObject.rb +++ b/testdata/ruby/imgObject.rb @@ -396,7 +396,7 @@ class IMG_TestClass < TestBase end - def test_4 + def test_5 tmp = File::join($ut_testtmp, "tmp.lyimg")