You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
When one registers a target, one is supposed to give a callback (comm_opened) for when a comm is opened by the frontend.
The comm_opened callback takes the xcomm as rvalue reference. If the backend expects the comm to be closed by the frontend, the lifetime is unknown and the comm C++ object has to be stored somewhere (in some sort of registry for instance).
When the frontend do close the comm, we want to delete the C++ xcomm object (and other related resources). xcomm::on_close can be used to set a callback on the xcomm when it is closed by the frontend.
However that callback is called in xcomm::handle_close (member function), which means that we would be calling ~xcomm from within the object.
voidcomm_opened(xeus::xcomm&& comm, const xeus::xmessage&)
{
// This is a very simple registry for comm since their lifetime is managed by the frontend static std::unordered_map<xeus::xguid, xeus::xcomm> comm_registry{};
auto iter_inserted = comm_registry.emplace(std::make_pair(comm.id(), std::move(comm)));
assert(iter_inserted.second);
auto& registered_comm = iter_inserted.first->second;
registered_comm.on_message(
[&](const ::xeus::xmessage&) { // Does something }
);
registered_comm.on_close(
[&](const ::xeus::xmessage&)
{
// The comm is destructed from within one of its member function.
comm_registry.erase(registered_comm.id());
}
);
}
Current status
The code above currently works because no other instructions are executed after calling the on_close callback in handle_close.
Is this well defined behavior though?
Furthermore, it is a bad design because if any of the user or a Xeus developer, unaware of that pattern, decides to add additional work after the on_close callback, this will break.
Proposed solutions
Same but more explicit
Keep the solution as it is, but make it more explicit and documented.
For instance there could be an additional callback without a xmessage argument to avoid confusion.
Problem
When one registers a target, one is supposed to give a callback (
comm_opened
) for when a comm is opened by the frontend.The
comm_opened
callback takes thexcomm
as rvalue reference. If the backend expects thecomm
to be closed by the frontend, the lifetime is unknown and the comm C++ object has to be stored somewhere (in some sort of registry for instance).When the frontend do close the comm, we want to delete the C++
xcomm
object (and other related resources).xcomm::on_close
can be used to set a callback on thexcomm
when it is closed by the frontend.However that callback is called in
xcomm::handle_close
(member function), which means that we would be calling~xcomm
from within the object.Current status
The code above currently works because no other instructions are executed after calling the
on_close
callback inhandle_close
.Is this well defined behavior though?
Furthermore, it is a bad design because if any of the user or a Xeus developer, unaware of that pattern, decides to add additional work after the
on_close
callback, this will break.Proposed solutions
Same but more explicit
Keep the solution as it is, but make it more explicit and documented.
For instance there could be an additional callback without a
xmessage
argument to avoid confusion.Manage (some) xcomm from within Xeus
We could imagine having a
xeus::get_interpreter().comm_manager().register_managed_comm_target
Where the
comm_opened
callback would take thexcomm
as a lvalue reference and Xeus would manage its lifetime.For Xwidgets created from the frontend (which hold a
xcomm
), it would mean they should only capture a reference to thexcomm
.The text was updated successfully, but these errors were encountered: