Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Qt widget mixin segfaults on unexpected messages. #973

Closed
alatdneg opened this issue Nov 15, 2019 · 9 comments · Fixed by #1564
Closed

Qt widget mixin segfaults on unexpected messages. #973

alatdneg opened this issue Nov 15, 2019 · 9 comments · Fixed by #1564
Assignees
Milestone

Comments

@alatdneg
Copy link
Contributor

Send it a unexpected message and it get's very unhappy, as soon as I add a placeholder to receive it it's happy again.

Is there special behaviour required here?

@Neverlord Neverlord added the bug label Nov 15, 2019
@Neverlord
Copy link
Member

Is there special behaviour required here?

No, at least it should report an error in this case but not producing a segfault.

@alatdneg
Copy link
Contributor Author

Yup died straight away, when I removed the handler for the message type.

#0  0x00007ffff7308280 in caf::actor_system::scheduler() () from /builds/actor_framework/0.17.2/cbb634ff98/lib64/libcaf_core.so.0.17.2
#1  0x00007ffff73e581d in caf::scoped_execution_unit::exec_later(caf::resumable*) ()
   from /builds/actor_framework/0.17.2/cbb634ff98/lib64/libcaf_core.so.0.17.2
#2  0x00007ffff73d3d9a in caf::scheduled_actor::enqueue(std::unique_ptr<caf::mailbox_element, caf::detail::disposer>, caf::execution_unit*) ()
   from /builds/actor_framework/0.17.2/cbb634ff98/lib64/libcaf_core.so.0.17.2
#3  0x00007ffff72efafb in caf::abstract_actor::enqueue(caf::intrusive_ptr<caf::actor_control_block>, caf::message_id, caf::message, caf::execution_unit*) () from /builds/actor_framework/0.17.2/cbb634ff98/lib64/libcaf_core.so.0.17.2
#4  0x00007ffff72f266d in caf::actor_control_block::enqueue(caf::intrusive_ptr<caf::actor_control_block>, caf::message_id, caf::message, caf::execution_unit*) () from /builds/actor_framework/0.17.2/cbb634ff98/lib64/libcaf_core.so.0.17.2
#5  0x00007ffff73d2768 in caf::response_promise::deliver_impl(caf::message) ()
   from /builds/actor_framework/0.17.2/cbb634ff98/lib64/libcaf_core.so.0.17.2
#6  0x00007ffff73d281f in caf::response_promise::deliver(caf::error) ()
   from /builds/actor_framework/0.17.2/cbb634ff98/lib64/libcaf_core.so.0.17.2
#7  0x00007ffff73dc299 in caf::detail::default_invoke_result_visitor<caf::scheduled_actor>::operator()(caf::error&) ()
   from /builds/actor_framework/0.17.2/cbb634ff98/lib64/libcaf_core.so.0.17.2
#8  0x00007ffff73d4519 in caf::scheduled_actor::consume(caf::mailbox_element&)::{lambda()#4}::operator()() const ()
   from /builds/actor_framework/0.17.2/cbb634ff98/lib64/libcaf_core.so.0.17.2
#9  0x00007ffff73d9d82 in caf::scheduled_actor::consume(caf::mailbox_element&) ()
   from /builds/actor_framework/0.17.2/cbb634ff98/lib64/libcaf_core.so.0.17.2
#10 0x00007ffff73da086 in caf::scheduled_actor::reactivate(caf::mailbox_element&) ()
   from /builds/actor_framework/0.17.2/cbb634ff98/lib64/libcaf_core.so.0.17.2
#11 0x00007ffff73da1bb in caf::scheduled_actor::activate(caf::execution_unit*, caf::mailbox_element&) ()
   from /builds/actor_framework/0.17.2/cbb634ff98/lib64/libcaf_core.so.0.17.2
#12 0x00000000007d8bae in caf::mixin::actor_widget<QWidget, 32337>::event (this=0xbb5310, event=0x7fffdc000a10)
    at /builds/actor_framework/0.17.2/cbb634ff98/include/caf/mixin/actor_widget.hpp:85
#13 0x00007ffff5c5e5a1 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /builds/qt/5.8.0/741413210f/lib/libQt5Widgets.so.5
#14 0x00007ffff5c65aea in QApplication::notify(QObject*, QEvent*) () from /builds/qt/5.8.0/741413210f/lib/libQt5Widgets.so.5
#15 0x00007ffff4fb03c1 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /builds/qt/5.8.0/741413210f/lib/libQt5Core.so.5
#16 0x00007ffff4fb235b in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) ()
   from /builds/qt/5.8.0/741413210f/lib/libQt5Core.so.5
#17 0x00007ffff4ffbb53 in ?? () from /builds/qt/5.8.0/741413210f/lib/libQt5Core.so.5
#18 0x00007ffff0b8c36e in g_main_dispatch (context=0x7ffec80016f0) at gmain.c:3234
#19 g_main_context_dispatch (context=context@entry=0x7ffec80016f0) at gmain.c:3899
#20 0x00007ffff0b8c5d8 in g_main_context_iterate (context=context@entry=0x7ffec80016f0, block=block@entry=1, dispatch=dispatch@entry=1, 
    self=<optimized out>) at gmain.c:3972
#21 0x00007ffff0b8c65c in g_main_context_iteration (context=0x7ffec80016f0, may_block=1) at gmain.c:4033
#22 0x00007ffff4ffbf47 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
   from /builds/qt/5.8.0/741413210f/lib/libQt5Core.so.5
#23 0x00007ffff4faed7b in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /builds/qt/5.8.0/741413210f/lib/libQt5Core.so.5
#24 0x00007ffff4fb63be in QCoreApplication::exec() () from /builds/qt/5.8.0/741413210f/lib/libQt5Core.so.5
#25 0x00000000006ec508 in launch_ui (argc=1, argv=0x7fffffff2468, system=...) at /u/al/dev/loupe_prototype/src/prototype/src/main.cpp:139
#26 0x00000000006ec9a3 in main (argc=1, argv=0x7fffffff2468) at /u/al/dev/loupe_prototype/src/prototype/src/main.cpp:195

drozhkov pushed a commit to drozhkov/actor-framework that referenced this issue Feb 7, 2022
drozhkov pushed a commit to drozhkov/actor-framework that referenced this issue Feb 8, 2022
riemass added a commit that referenced this issue Sep 14, 2023
@riemass
Copy link
Member

riemass commented Sep 14, 2023

Hi @alatdneg. Thanks for providing test code at your personal branch. The unexpected message log line you saw is common behavior in case of receiving something you don't have a message handler for, and is unrelated to the segfault.

The issue you mentioned goes away once you include the caf::id_block::qtsupport in the CAF_MAIN macro, like you've written yourself at the bottom: CAF_MAIN(caf::id_block::qtsupport, caf::io::middleman).
Listing the id_block is mandatory when using custom message blocks. Can you confirm that was the issue all along?

To discuss this further:

@Neverlord receiving a message from an unknown id_block causes a segfault at the destructor of message_data. This behavior is only present in case of a mixin::actor_widget. Given an event based actor with the same setup, it won't segfault. Should we try avoiding the segfault at all costs, or do we keep it as is and restrict the users to include the type id?

Here is the example.

@alatdneg
Copy link
Contributor Author

alatdneg commented Sep 15, 2023

Ah ta, I see, yeah that'd probably work, as my atoms are used external of Qt as well as in it, i'd have to apply the same default offset.

Custom Qt events start at 1000. So I'll have to offset my first atom id by that amount, that way I only need to declare one set of atoms, not two sets (internal vs Qt Event flavour with the 1000 id offset .

Does that sound right ? Or is there only one atom id used in the Qt event, as it wraps the message in the event. Sorry it's been awhile since I've touched this code and forgotten how it works :)

@alatdneg
Copy link
Contributor Author

It shouldn't segfault, as that could be triggered accidentally by a remote actor sending an unknown message id ?

@riemass
Copy link
Member

riemass commented Sep 19, 2023

Ah ta, I see, yeah that'd probably work, as my atoms are used external of Qt as well as in it, i'd have to apply the same default offset.

The second parameter to CAF_BEGIN_TYPE_ID_BLOCK is the first type_id from where your custom messages begin, you shouldn't violate ODR here and keep it consistent across actor applications, even distributed ones. Also you have to keep track not to have two different types under the same type id.

Custom Qt events start at 1000. So I'll have to offset my first atom id by that amount, that way I only need to declare one set of atoms, not two sets (internal vs Qt Event flavour with the 1000 id offset .

Does that sound right ? Or is there only one atom id used in the Qt event, as it wraps the message in the event. Sorry it's been awhile since I've touched this code and forgotten how it works :)

If I've got the question right here, then yes, you declare one set of atoms and use those across the whole application. It makes no difference if you make a Qt widget or an event_based_actor. Otherwise, the runtime would threat atoms with different type_id's as different types and you would run into serialization issues. As your use case grows, best would be to isolate the custom message types to a separate hpp/cpp unit. This would also help if you have multiple executables.

@Neverlord
Copy link
Member

Ah ta, I see, yeah that'd probably work, as my atoms are used external of Qt as well as in it, i'd have to apply the same default offset.

Custom Qt events start at 1000. So I'll have to offset my first atom id by that amount, that way I only need to declare one set of atoms, not two sets (internal vs Qt Event flavour with the 1000 id offset .

Just a note on this: using the same ID will work. Just be aware that CAF allocates an array to hold the type ID information. So large gaps will mean some waste of memory and you'll have an array with > 1000 entries. Shouldn't really matter unless you're running in constrained (embedded) devices, but I wanted to point it out. :)

Does that sound right ? Or is there only one atom id used in the Qt event, as it wraps the message in the event. Sorry it's been awhile since I've touched this code and forgotten how it works :)

As long as you register the ID in both worlds, I don't see a reason for concerns.

It shouldn't segfault, as that could be triggered accidentally by a remote actor sending an unknown message id ?

When sending messages over the wire, CAF will check incoming type IDs and will complain to the sender of the message when receiving an unknown type ID. Initializing the local meta information table is always required, though.

Sorry for the inconvenience. With 0.19, we have already added diagnostic to print a useful error message when not initializing the meta object table (https://github.com/actor-framework/actor-framework/blob/master/CHANGELOG.md#0190---2023-04-17) to make it a lot more obvious what's going wrong in cases like this. However, this error message isn't showing up here. We'll improve on the error reporting.

@alatdneg
Copy link
Contributor Author

Ah, cheers I didn't realise it allocated the array fully, I had a quite sizeable offset in there..

@Neverlord
Copy link
Member

Changing the label, since it's actually a lack of diagnostic, not a bug per se.

@Neverlord Neverlord added this to the 0.19.3 milestone Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants