Fix: Compositor leaks memory in certain setups

The GPU and new CPU compositors leak memory when an output is connected
to multiple inputs inside the same pixel operation. That's because nodes
do not know that their multiple outgoing links are in fact going to the
same operation, so their initial reference count is more than the actual
reference count.

To fix this, we keep track of the reference count of pixel operation
inputs and release the inputs based on that reference count.

source/blender/compositor/realtime_compositor/COM_operation.hh

@@ -159,6 +159,11 @@ class Operation {
   /* Get a reference to the descriptor of the input identified by the given identified. */
   InputDescriptor &get_input_descriptor(StringRef identifier);
 
+  /* Release the results that are mapped to the inputs of the operation. This is called after the
+   * evaluation of the operation to declare that the results are no longer needed by this
+   * operation. */
+  virtual void release_inputs();
+
   /* Returns a reference to the compositor context. */
   Context &context() const;
 
@@ -174,11 +179,6 @@ class Operation {
    * information. */
   void reset_results();
 
-  /* Release the results that are mapped to the inputs of the operation. This is called after the
-   * evaluation of the operation to declare that the results are no longer needed by this
-   * operation. */
-  void release_inputs();
-
   /* Release the results that were allocated in the execute method but are not actually needed.
    * This can be the case if the execute method allocated a dummy texture for an unneeded result,
    * see the description of Result::allocate_texture() for more information. This is called after

source/blender/compositor/realtime_compositor/COM_pixel_operation.hh

@@ -77,6 +77,12 @@ class PixelOperation : public Operation {
   /* A map that associates the output socket of a node that is not part of the pixel operation to
    * the identifier of the input of the operation that was declared for it. */
   Map<DOutputSocket, std::string> outputs_to_declared_inputs_map_;
+  /* A map that associates the identifier of each input of the operation with the number of node
+   * inputs that use it, that is, its reference count. This is needed to properly release inputs
+   * after evaluation, since the results that provide the inputs aren't aware that multiple of
+   * their outgoing links are now part of a single pixel operation, so we need to release the
+   * inputs with their internal reference count stored here. See PixelOperation::release_inputs. */
+  Map<std::string, int> inputs_to_reference_counts_map_;
   /* A map that associates the output socket that provides the result of an output of the operation
    * with the identifier of that output. This is needed to help the compiler establish links
    * between operations. */
@@ -128,6 +134,12 @@ class PixelOperation : public Operation {
    *
    * The node execution schedule is given as an input. */
   void compute_results_reference_counts(const Schedule &schedule);
+
+ protected:
+  /* Release the results that are mapped to the inputs of the operation, making sure to release the
+   * input with a count equivalent to its reference count stored in inputs_to_reference_counts_map_
+   * to avoid leaking the result. */
+  void release_inputs() override;
 };
 
 }  // namespace blender::realtime_compositor

source/blender/compositor/realtime_compositor/COM_result.hh

@@ -341,10 +341,10 @@ class Result {
    * result, the reference count of the master result is incremented instead. */
   void increment_reference_count(int count = 1);
 
-  /* Decrement the reference count of the result and free its texture if the reference count
-   * reaches zero. This should be called when an operation that used this result no longer needs
-   * it. If this result have a master result, the master result is released instead. */
-  void release();
+  /* Decrement the reference count of the result by the given count and free its data if it reaches
+   * zero. The given count should not be more than the current reference count. If this result have
+   * a master result, the master result is released instead. */
+  void release(const int count = 1);
 
   /* Frees the result data. If the result is not allocated or wraps external data, then this does
    * nothing. If this result have a master result, the master result is freed instead. */

source/blender/compositor/realtime_compositor/intern/multi_function_procedure_operation.cc

@@ -237,17 +237,21 @@ mf::Variable *MultiFunctionProcedureOperation::get_multi_function_input_variable
     DInputSocket input_socket, DOutputSocket output_socket)
 {
   /* An input was already declared for that same output socket, so no need to declare it again and
-   * we just return its variable. But we update the domain priority of the input descriptor to be
-   * the higher priority of the existing descriptor and the descriptor of the new input socket.
-   * That's because the same output might be connected to multiple inputs inside the multi-function
-   * procedure operation which have different priorities. */
+   * we just return its variable.  */
   if (output_to_variable_map_.contains(output_socket)) {
+    /* But first we update the domain priority of the input descriptor to be the higher priority of
+     * the existing descriptor and the descriptor of the new input socket. That's because the same
+     * output might be connected to multiple inputs inside the multi-function procedure operation
+     * which have different priorities. */
     const std::string input_identifier = outputs_to_declared_inputs_map_.lookup(output_socket);
     InputDescriptor &input_descriptor = this->get_input_descriptor(input_identifier);
     input_descriptor.domain_priority = math::min(
         input_descriptor.domain_priority,
         input_descriptor_from_input_socket(input_socket.bsocket()).domain_priority);
 
+    /* Increment the input's reference count. */
+    inputs_to_reference_counts_map_.lookup(input_identifier)++;
+
     return output_to_variable_map_.lookup(output_socket);
   }
 
@@ -274,6 +278,10 @@ mf::Variable *MultiFunctionProcedureOperation::get_multi_function_input_variable
   /* Map the output socket to the identifier of the operation input that was declared for it. */
   outputs_to_declared_inputs_map_.add_new(output_socket, input_identifier);
 
+  /* Map the identifier of the operation input to a reference count of 1, this will later be
+   * incremented if that same output was referenced again. */
+  inputs_to_reference_counts_map_.add_new(input_identifier, 1);
+
   return &variable;
 }
 

source/blender/compositor/realtime_compositor/intern/operation.cc

@@ -173,6 +173,15 @@ InputDescriptor &Operation::get_input_descriptor(StringRef identifier)
   return input_descriptors_.lookup(identifier);
 }
 
+void Operation::release_unneeded_results()
+{
+  for (Result &result : results_.values()) {
+    if (!result.should_compute() && result.is_allocated()) {
+      result.release();
+    }
+  }
+}
+
 Context &Operation::context() const
 {
   return context_;
@@ -212,13 +221,4 @@ void Operation::release_inputs()
   }
 }
 
-void Operation::release_unneeded_results()
-{
-  for (Result &result : results_.values()) {
-    if (!result.should_compute() && result.is_allocated()) {
-      result.release();
-    }
-  }
-}
-
 }  // namespace blender::realtime_compositor

source/blender/compositor/realtime_compositor/intern/pixel_operation.cc

@@ -93,4 +93,11 @@ void PixelOperation::compute_results_reference_counts(const Schedule &schedule)
   }
 }
 
+void PixelOperation::release_inputs()
+{
+  for (const auto item : inputs_to_reference_counts_map_.items()) {
+    this->get_input(item.key).release(item.value);
+  }
+}
+
 }  // namespace blender::realtime_compositor

source/blender/compositor/realtime_compositor/intern/result.cc

@@ -627,17 +627,19 @@ void Result::increment_reference_count(int count)
   reference_count_ += count;
 }
 
-void Result::release()
+void Result::release(const int count)
 {
+  BLI_assert(count > 0);
+
   /* If there is a master result, release it instead. */
   if (master_) {
-    master_->release();
+    master_->release(count);
     return;
   }
 
-  /* Decrement the reference count, and if it reaches zero, release the texture back into the
-   * texture pool. */
-  reference_count_--;
+  /* Decrement the reference count, and if it is not yet zero, return and do not free. */
+  reference_count_ -= count;
+  BLI_assert(reference_count_ >= 0);
   if (reference_count_ != 0) {
     return;
   }

source/blender/compositor/realtime_compositor/intern/shader_operation.cc

@@ -189,6 +189,9 @@ void ShaderOperation::link_node_input_external(DInputSocket input_socket,
     input_descriptor.domain_priority = math::min(
         input_descriptor.domain_priority,
         input_descriptor_from_input_socket(input_socket.bsocket()).domain_priority);
+
+    /* Increment the input's reference count. */
+    inputs_to_reference_counts_map_.lookup(input_identifier)++;
   }
 
   /* Link the attribute representing the shader operation input corresponding to the given output
@@ -244,6 +247,10 @@ void ShaderOperation::declare_operation_input(DInputSocket input_socket,
 
   /* Map the output socket to the identifier of the operation input that was declared for it. */
   outputs_to_declared_inputs_map_.add_new(output_socket, input_identifier);
+
+  /* Map the identifier of the operation input to a reference count of 1, this will later be
+   * incremented if that same output was referenced again. */
+  inputs_to_reference_counts_map_.add_new(input_identifier, 1);
 }
 
 void ShaderOperation::populate_results_for_node(DNode node)