[BUG] Inconsistent Output in Demo

[Sfinx]

The first event is never printed. This is because following do not work:

first_event evt{ "hello from dummy event" };
evt_bus.fire_event(&evt);

but following works:

evt_bus.fire_event(third_event{ 13.0 });

And test SIGSEGV's at aarch64 in DeregisterWhileDispatching

[DeveloperPaul123]

Hi, thank you for the bug report.

Can you provide the entire error output as well as info on your configuration? What operating system are you using?

[Sfinx]

Stock Ubuntu 20.04. The demo outputs the following:

I just do stuff when a third_event is fired.
my third event handler: 13
I just do stuff when a third_event is fired.
third event handler says: oh boy...
event: oh boy...
Firing other event
second other event handler says: 2 hello there
first other event handler says: hello there
Firing other event
second other event handler says: 2 hello there
I just do stuff when a third_event is fired.
callback count: 1
handler count: 6
removing handlers...

So no first event message

[DeveloperPaul123]

What compiler are you using?

[DeveloperPaul123]

@Sfinx I believe I fixed the issue in 8375a7d. Can you give it a try?

[Sfinx]

As already said - this works when pushing all the object to stack - but it is still do not works when passing by reference. The correct fix must work for both. As pushing the only object is somewhat ugly.

P.S. I'm using gcc-11 and this needed to compile:

diff --git a/eventbus/include/eventbus/event_bus.hpp b/eventbus/include/eventbus/event_bus.hpp
index 56c665d..7165687 100644
--- a/eventbus/include/eventbus/event_bus.hpp
+++ b/eventbus/include/eventbus/event_bus.hpp
@@ -10,6 +10,7 @@
 #include <atomic>
 #include <thread>
 #include <utility>
+#include <mutex>
 
 namespace dp
 {

And there is compile warnings:

n file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp: In function ‘int main()’:
eventbus/eventbus/include/eventbus/event_bus.hpp:229:19: warning: ‘handle’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  229 |                 : handle_(handle), event_bus_(bus)
      |                   ^~~~~~~~~~~~~~~
In file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp:47:37: note: ‘handle’ was declared here
   47 |                         const void* handle;
      |                                     ^~~~~~
In file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp:229:19: warning: ‘handle’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  229 |                 : handle_(handle), event_bus_(bus)
      |                   ^~~~~~~~~~~~~~~
In file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp:47:37: note: ‘handle’ was declared here
   47 |                         const void* handle;
      |                                     ^~~~~~
In file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp:229:19: warning: ‘handle’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  229 |                 : handle_(handle), event_bus_(bus)
      |                   ^~~~~~~~~~~~~~~
In file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp:47:37: note: ‘handle’ was declared here
   47 |                         const void* handle;
      |                                     ^~~~~~
In file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp:229:19: warning: ‘handle’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  229 |                 : handle_(handle), event_bus_(bus)
      |                   ^~~~~~~~~~~~~~~
In file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp:47:37: note: ‘handle’ was declared here
   47 |                         const void* handle;
      |                                     ^~~~~~
In file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp:229:19: warning: ‘handle’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  229 |                 : handle_(handle), event_bus_(bus)
      |                   ^~~~~~~~~~~~~~~
In file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp:47:37: note: ‘handle’ was declared here
   47 |                         const void* handle;
      |                                     ^~~~~~

[Sfinx]

And there is design problem concerning return value of register_handler() - assuming you have the class:

class test {
 public:
  test() {
    auto reg = evt_bus.register_handler();
  }
 ~test() {
    // how to remove the handler ???
 }
};

The first problem is that if reg var is scope-auto-deleted - the handler will never invoke. I think that scope problems of return value can never affect the exact function. This is confusing behavior at least.
The second problem is that standard approach initializing the reg reference in constructor do not work too so no way to remove the handler on-demand. The only way is to use "static" for reg which is really ugly too.

[DeveloperPaul123]

As already said - this works when pushing all the object to stack - but it is still do not works when passing by reference. The correct fix must work for both. As pushing the only object is somewhat ugly.

In the demo, it was not passing by reference, it was passing a pointer of first_event to the fire_event function. At this time, I don't really want to support pointers as event types. This is why the event was not being called, because in the demo it was passing first_event* as the event type, and no registration handlers were available that handle events of that type.

P.S. I'm using gcc-11 and this needed to compile:

diff --git a/eventbus/include/eventbus/event_bus.hpp b/eventbus/include/eventbus/event_bus.hpp
index 56c665d..7165687 100644
--- a/eventbus/include/eventbus/event_bus.hpp
+++ b/eventbus/include/eventbus/event_bus.hpp
@@ -10,6 +10,7 @@
 #include <atomic>
 #include <thread>
 #include <utility>
+#include <mutex>

Noted and fixed.

And there is compile warnings:

n file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp: In function ‘int main()’:
eventbus/eventbus/include/eventbus/event_bus.hpp:229:19: warning: ‘handle’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  229 |                 : handle_(handle), event_bus_(bus)
      |                   ^~~~~~~~~~~~~~~
In file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp:47:37: note: ‘handle’ was declared here
   47 |                         const void* handle;
      |                                     ^~~~~~
In file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp:229:19: warning: ‘handle’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  229 |                 : handle_(handle), event_bus_(bus)
      |                   ^~~~~~~~~~~~~~~
In file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp:47:37: note: ‘handle’ was declared here
   47 |                         const void* handle;
      |                                     ^~~~~~
In file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp:229:19: warning: ‘handle’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  229 |                 : handle_(handle), event_bus_(bus)
      |                   ^~~~~~~~~~~~~~~
In file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp:47:37: note: ‘handle’ was declared here
   47 |                         const void* handle;
      |                                     ^~~~~~
In file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp:229:19: warning: ‘handle’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  229 |                 : handle_(handle), event_bus_(bus)
      |                   ^~~~~~~~~~~~~~~
In file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp:47:37: note: ‘handle’ was declared here
   47 |                         const void* handle;
      |                                     ^~~~~~
In file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp:229:19: warning: ‘handle’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  229 |                 : handle_(handle), event_bus_(bus)
      |                   ^~~~~~~~~~~~~~~
In file included from eventbus/demo/main.cpp:4:
eventbus/eventbus/include/eventbus/event_bus.hpp:47:37: note: ‘handle’ was declared here
   47 |                         const void* handle;
      |                                     ^~~~~~

I do not see these warnings with GCC 11 on Ubuntu 20.04. Have you changed the compilation flags?

[Sfinx]

CFLAGS are "-Wall -O2"

[DeveloperPaul123]

And there is design problem concerning return value of register_handler() - assuming you have the class:

class test {
 public:
  test() {
    auto reg = evt_bus.register_handler();
  }
 ~test() {
    // how to remove the handler ???
 }
};

The first problem is that if reg var is scope-auto-deleted - the handler will never invoke. I think that scope problems of return value can never affect the exact function. This is confusing behavior at least. The second problem is that standard approach initializing the reg reference in constructor do not work too so no way to remove the handler on-demand. The only way is to use "static" for reg which is really ugly too.

The handler_registration class is move constructible, so you can in fact initialize it in the constructor like so:

class test_class {
    dp::handler_registration reg;
public:
    test_class(dp::event_bus& bus) : reg(std::move(bus.register_handler<first_event>([](const first_event& evt) {
        std::cout << "test class: " << evt.message << "\n";
        }))) {

    }
};

The registration will automatically de-register when test_class goes out of scope.

[Sfinx]

Thanks, will try