fix: node graph edge display, teardown crashes, and event/audio lifetime bugs
- capi: oakengine_node_output_connection_at/_at_ex returned the source node as the connection destination; the actual destination is conn.second.node(). Out-edge enumeration was useless, so the node view could only draw in-edges and randomly lost whichever edges needed the out-edge path (random per context build order). Regression test in oakengine_node_test - project teardown: Project::clear() pre-notifies node removal while nodes are fully constructed (observers used to crash on half-destroyed nodes); childEvent suppresses the removal dance while clearing; Node::disconnect_all/disconnect_edge get a silent mode for teardown so no invalidation/events touch dying members (is_being_cleared); ClipBlock marker disconnect guarded against dead viewer/markers; ProjectCopier and PreviewAutoCacher drop project references on Project::destroyed instead of disconnecting dead objects at shutdown - preview: add oakengine_preview_request_get_audio_sample_count; the viewer queried sample count by passing nullptr to get_audio_samples which rejects it, so all playback audio was silently dropped - app: fix unterminated input-id memcpy in ResolveGroupInput (nodeparamviewitem, widgetbridge) that corrupted every parameter id - app: unsubscribe raw C-API event subscriptions in destructors of NodeParamViewKeyframeControl, NodeParamViewConnectedLabel and ExportDialog; playhead events used to fire into dead widgets (crash when dragging the playhead) - tests: preview request roundtrip (video frame + audio range) and free-while-active teardown coverage; env-gated OAK_DEBUG_EDGES / OAK_DEBUG_INVALID_INPUT diagnostics - docs: investigation notes in docs/zh/
This commit is contained in:
@@ -435,30 +435,36 @@ void ClipBlock::invalidate_cache(const TimeRange &range, const QString &from,
|
||||
viewers.isEmpty() ? nullptr : viewers.first();
|
||||
|
||||
if (new_connected_viewer != connected_viewer_) {
|
||||
TimelineMarkerList *old_markers =
|
||||
connected_viewer_ ? connected_viewer_->get_markers() : nullptr;
|
||||
if (old_markers) {
|
||||
disconnect(old_markers, &TimelineMarkerList::marker_added, this,
|
||||
&ClipBlock::preview_changed);
|
||||
disconnect(old_markers, &TimelineMarkerList::marker_removed,
|
||||
this, &ClipBlock::preview_changed);
|
||||
disconnect(old_markers, &TimelineMarkerList::marker_modified,
|
||||
this, &ClipBlock::preview_changed);
|
||||
}
|
||||
if (connected_viewer_) {
|
||||
disconnect(connected_viewer_->get_markers(),
|
||||
&TimelineMarkerList::marker_added, this,
|
||||
&ClipBlock::preview_changed);
|
||||
disconnect(connected_viewer_->get_markers(),
|
||||
&TimelineMarkerList::marker_removed, this,
|
||||
&ClipBlock::preview_changed);
|
||||
disconnect(connected_viewer_->get_markers(),
|
||||
&TimelineMarkerList::marker_modified, this,
|
||||
&ClipBlock::preview_changed);
|
||||
disconnect(connected_viewer_, &ViewerOutput::destroyed, this,
|
||||
nullptr);
|
||||
}
|
||||
|
||||
connected_viewer_ = new_connected_viewer;
|
||||
|
||||
TimelineMarkerList *new_markers =
|
||||
connected_viewer_ ? connected_viewer_->get_markers() : nullptr;
|
||||
if (new_markers) {
|
||||
connect(new_markers, &TimelineMarkerList::marker_added, this,
|
||||
&ClipBlock::preview_changed);
|
||||
connect(new_markers, &TimelineMarkerList::marker_removed, this,
|
||||
&ClipBlock::preview_changed);
|
||||
connect(new_markers, &TimelineMarkerList::marker_modified,
|
||||
this, &ClipBlock::preview_changed);
|
||||
}
|
||||
if (connected_viewer_) {
|
||||
connect(connected_viewer_->get_markers(),
|
||||
&TimelineMarkerList::marker_added, this,
|
||||
&ClipBlock::preview_changed);
|
||||
connect(connected_viewer_->get_markers(),
|
||||
&TimelineMarkerList::marker_removed, this,
|
||||
&ClipBlock::preview_changed);
|
||||
connect(connected_viewer_->get_markers(),
|
||||
&TimelineMarkerList::marker_modified, this,
|
||||
&ClipBlock::preview_changed);
|
||||
connect(connected_viewer_, &ViewerOutput::destroyed, this,
|
||||
[this]() { connected_viewer_ = nullptr; });
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -265,6 +265,10 @@ private:
|
||||
TransitionBlock *in_transition_;
|
||||
TransitionBlock *out_transition_;
|
||||
|
||||
// Cleared via the viewer's destroyed() signal (see invalidate_cache);
|
||||
// during project teardown the viewer can die before this clip, and a
|
||||
// raw dangling pointer here led to disconnecting signals on a dead
|
||||
// TimelineMarkerList.
|
||||
ViewerOutput *connected_viewer_;
|
||||
|
||||
private:
|
||||
|
||||
+48
-3
@@ -21,6 +21,8 @@
|
||||
|
||||
#include "node.h"
|
||||
|
||||
#include <execinfo.h>
|
||||
|
||||
#include <QApplication>
|
||||
#include <QGuiApplication>
|
||||
#include <QDebug>
|
||||
@@ -216,7 +218,7 @@ void Node::connect_edge(Node *output, const NodeInput &input)
|
||||
}
|
||||
}
|
||||
|
||||
void Node::disconnect_edge(Node *output, const NodeInput &input)
|
||||
void Node::disconnect_edge(Node *output, const NodeInput &input, bool silent)
|
||||
{
|
||||
// Ensure graph is the same
|
||||
Q_ASSERT(input.node()->parent() == output->parent());
|
||||
@@ -232,6 +234,18 @@ void Node::disconnect_edge(Node *output, const NodeInput &input)
|
||||
outputs.erase(std::find(outputs.begin(), outputs.end(),
|
||||
std::pair<Node *, NodeInput>({ output, input })));
|
||||
|
||||
if (silent) {
|
||||
// Teardown-only mode: just drop the edge from both maps. No events,
|
||||
// no signals, no invalidation — the whole graph is being destroyed
|
||||
// and handlers would touch half-destroyed nodes.
|
||||
return;
|
||||
}
|
||||
|
||||
if (qEnvironmentVariableIsSet("OAK_DEBUG_EDGES")) {
|
||||
qWarning("EDGE-DEBUG: disconnect_edge %p -> %p (%s)", (void *)output,
|
||||
(void *)input.node(), qPrintable(input.input()));
|
||||
}
|
||||
|
||||
// Call internal events
|
||||
input.node()->InputDisconnectedEvent(input.input(), input.element(),
|
||||
output);
|
||||
@@ -1239,6 +1253,14 @@ Node *Node::copy_node_in_graph(Node *node, MultiUndoCommand *command)
|
||||
void Node::send_invalidate_cache(const TimeRange &range,
|
||||
const InvalidateCacheOptions &options)
|
||||
{
|
||||
// During project teardown, don't propagate invalidation across edges:
|
||||
// the nodes on the other side may already be half-destroyed.
|
||||
if (Project *p = project()) {
|
||||
if (p->is_being_cleared()) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
for (const OutputConnection &conn : output_connections_) {
|
||||
// Send clear cache signal to the Node
|
||||
const NodeInput &in = conn.second;
|
||||
@@ -1249,6 +1271,12 @@ void Node::send_invalidate_cache(const TimeRange &range,
|
||||
|
||||
void Node::invalidate_all(const QString &input, int element)
|
||||
{
|
||||
if (Project *p = project()) {
|
||||
if (p->is_being_cleared()) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
invalidate_cache(TimeRange(RATIONAL_MIN, RATIONAL_MAX), input, element);
|
||||
}
|
||||
|
||||
@@ -1910,6 +1938,18 @@ void Node::report_invalid_input(const char *attempted_action, const QString &id,
|
||||
qWarning()
|
||||
<< "Failed to" << attempted_action << "parameter" << id << "element"
|
||||
<< element << "in node" << this->id() << "- input doesn't exist";
|
||||
|
||||
if (qEnvironmentVariableIsSet("OAK_DEBUG_INVALID_INPUT")) {
|
||||
void *frames[32];
|
||||
const int n = backtrace(frames, 32);
|
||||
char **symbols = backtrace_symbols(frames, n);
|
||||
if (symbols) {
|
||||
for (int i = 0; i < n; i++) {
|
||||
qWarning("INVALID-INPUT-BT: %s", symbols[i]);
|
||||
}
|
||||
free(symbols);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
NodeInputImmediate *Node::create_immediate(const QString &input)
|
||||
@@ -2284,15 +2324,20 @@ bool Node::inputs_from(const QString &id, bool recursively) const
|
||||
|
||||
void Node::disconnect_all()
|
||||
{
|
||||
// During project teardown, skip the disconnect events/signals: the
|
||||
// other side's handlers touch this node's members which are already
|
||||
// destroyed at ~Node time (e.g. caches), which is a use-after-free.
|
||||
const bool silent = project() && project()->is_being_cleared();
|
||||
|
||||
// Disconnect inputs (copy map since internal map will change as we disconnect)
|
||||
InputConnections copy = input_connections_;
|
||||
for (auto it = copy.cbegin(); it != copy.cend(); it++) {
|
||||
disconnect_edge(it->second, it->first);
|
||||
disconnect_edge(it->second, it->first, silent);
|
||||
}
|
||||
|
||||
while (!output_connections_.empty()) {
|
||||
OutputConnection conn = output_connections_.back();
|
||||
disconnect_edge(conn.first, conn.second);
|
||||
disconnect_edge(conn.first, conn.second, silent);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+2
-1
@@ -417,7 +417,8 @@ public:
|
||||
|
||||
static void connect_edge(Node *output, const NodeInput &input);
|
||||
|
||||
static void disconnect_edge(Node *output, const NodeInput &input);
|
||||
static void disconnect_edge(Node *output, const NodeInput &input,
|
||||
bool silent = false);
|
||||
|
||||
void copy_cache_uuids_from(Node *n);
|
||||
|
||||
|
||||
@@ -134,6 +134,12 @@ NodeEdgeAddCommand::~NodeEdgeAddCommand()
|
||||
|
||||
void NodeEdgeAddCommand::redo()
|
||||
{
|
||||
if (qEnvironmentVariableIsSet("OAK_DEBUG_EDGES")) {
|
||||
qWarning("EDGE-DEBUG: NodeEdgeAddCommand::redo %p -> %p (%s)",
|
||||
(void *)output_, (void *)input_.node(),
|
||||
qPrintable(input_.input()));
|
||||
}
|
||||
|
||||
if (input_.is_connected()) {
|
||||
if (!remove_command_) {
|
||||
remove_command_ =
|
||||
@@ -148,6 +154,12 @@ void NodeEdgeAddCommand::redo()
|
||||
|
||||
void NodeEdgeAddCommand::undo()
|
||||
{
|
||||
if (qEnvironmentVariableIsSet("OAK_DEBUG_EDGES")) {
|
||||
qWarning("EDGE-DEBUG: NodeEdgeAddCommand::undo %p -> %p (%s)",
|
||||
(void *)output_, (void *)input_.node(),
|
||||
qPrintable(input_.input()));
|
||||
}
|
||||
|
||||
Node::disconnect_edge(output_, input_);
|
||||
|
||||
if (remove_command_) {
|
||||
@@ -169,11 +181,23 @@ NodeEdgeRemoveCommand::NodeEdgeRemoveCommand(Node *output,
|
||||
|
||||
void NodeEdgeRemoveCommand::redo()
|
||||
{
|
||||
if (qEnvironmentVariableIsSet("OAK_DEBUG_EDGES")) {
|
||||
qWarning("EDGE-DEBUG: NodeEdgeRemoveCommand::redo %p -> %p (%s)",
|
||||
(void *)output_, (void *)input_.node(),
|
||||
qPrintable(input_.input()));
|
||||
}
|
||||
|
||||
Node::disconnect_edge(output_, input_);
|
||||
}
|
||||
|
||||
void NodeEdgeRemoveCommand::undo()
|
||||
{
|
||||
if (qEnvironmentVariableIsSet("OAK_DEBUG_EDGES")) {
|
||||
qWarning("EDGE-DEBUG: NodeEdgeRemoveCommand::undo %p -> %p (%s)",
|
||||
(void *)output_, (void *)input_.node(),
|
||||
qPrintable(input_.input()));
|
||||
}
|
||||
|
||||
Node::connect_edge(output_, input_);
|
||||
}
|
||||
|
||||
|
||||
@@ -86,6 +86,23 @@ void Project::initialize()
|
||||
|
||||
void Project::clear()
|
||||
{
|
||||
is_being_cleared_ = true;
|
||||
|
||||
// Notify observers about each node while it is still fully constructed.
|
||||
// The childEvent() removal path fires from ~Node, when derived-class
|
||||
// members are already destroyed; observers touching the node there
|
||||
// (e.g. querying a Sequence's tracks) used to crash.
|
||||
const QVector<Node *> notify_copy = node_children_;
|
||||
for (Node *node : notify_copy) {
|
||||
if (!node_children_.contains(node)) {
|
||||
// An observer already removed it from the graph
|
||||
continue;
|
||||
}
|
||||
emit node_removed(node);
|
||||
emit node->removed_from_graph(this);
|
||||
node->RemovedFromGraphEvent(this);
|
||||
}
|
||||
|
||||
// By deleting the last nodes first, we assume that nodes that are most important are deleted last
|
||||
// (e.g. Project's ColorManager or ProjectSettingsNode.
|
||||
for (auto it = node_children_.cbegin(); it != node_children_.cend(); it++) {
|
||||
@@ -95,6 +112,8 @@ void Project::clear()
|
||||
while (!node_children_.isEmpty()) {
|
||||
delete node_children_.last();
|
||||
}
|
||||
|
||||
is_being_cleared_ = false;
|
||||
}
|
||||
|
||||
SerializedData Project::load(QXmlStreamReader *reader)
|
||||
@@ -369,6 +388,14 @@ void Project::childEvent(QChildEvent *event)
|
||||
} else if (event->type() == QEvent::ChildRemoved) {
|
||||
node_children_.removeOne(node);
|
||||
|
||||
if (is_being_cleared_) {
|
||||
// Teardown: everything is being destroyed anyway. The
|
||||
// disconnect/emit dance below touches the half-destroyed
|
||||
// child (this fires from ~Node) and has repeatedly crashed;
|
||||
// Qt auto-disconnects the rest when destruction completes.
|
||||
return;
|
||||
}
|
||||
|
||||
// Disconnect signals
|
||||
disconnect(node, &Node::input_connected, this,
|
||||
&Project::input_connected);
|
||||
|
||||
@@ -103,6 +103,18 @@ public:
|
||||
}
|
||||
void set_modified(bool e);
|
||||
|
||||
/**
|
||||
* @brief True while clear()/teardown is deleting all nodes.
|
||||
*
|
||||
* Nodes use this to suppress cache invalidation storms while the graph
|
||||
* is being torn down: invalidating during destruction walks edges into
|
||||
* half-destroyed nodes and has repeatedly caused use-after-free crashes.
|
||||
*/
|
||||
bool is_being_cleared() const
|
||||
{
|
||||
return is_being_cleared_;
|
||||
}
|
||||
|
||||
bool has_autorecovery_been_saved() const;
|
||||
void set_autorecovery_saved(bool e);
|
||||
|
||||
@@ -259,6 +271,8 @@ private:
|
||||
|
||||
bool autorecovery_saved_;
|
||||
|
||||
bool is_being_cleared_ = false;
|
||||
|
||||
ColorManager *color_manager_;
|
||||
|
||||
QVector<Node *> node_children_;
|
||||
|
||||
Reference in New Issue
Block a user