From: Pawel Wasowski <p.wasowski2@samsung.com>
Date: Thu, 25 Mar 2021 10:42:12 +0000 (+0100)
Subject: [ML][Pipeline] Prevent a crash in ~CustomFilter and unregister failures
X-Git-Tag: submit/tizen/20210326.074032^0
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=a6cf78cbb82d7214734481f149c12447d6abaa4a;p=platform%2Fcore%2Fapi%2Fwebapi-plugins.git

[ML][Pipeline] Prevent a crash in ~CustomFilter and unregister failures

This commit fixes a crash that used to follow the sequence below:
1. ~CustomFilter() is called, but native
ml_pipeline_custom_easy_filter_unregister()
fails to unregister the filter. CustomFilter is
removed, but a pointer to this object (now defunct)
is still kept in CustomFilter::valid_filters_ container.
2. LockRequestIdToJSResponseMutexIfFilterIsNotWaitingForJSResponses,
that iterates over valid_filters_ accesses the destroyed object
and thus crashes the app.

This commit also changes the order of destruction of Pipeline and
CustomFilter objects managed by Web API. Pipeline objects should
be destroyed first.

[Verification] Tested in Chrome DevTools with the snippets below, works
fine.

inputTI = new tizen.ml.TensorsInfo();
inputTI.addTensorInfo("3D", "UINT8", [4, 20, 15, 1]);
outputTI = new tizen.ml.TensorsInfo();
outputTI.addTensorInfo("flat", "UINT8", [1200]);

filterCB = function (input, output) {
  console.log('hello');
  tizen.ml.pipeline.unregisterCustomFilter("flattenFilter");
  console.log('bye');
}

retValue = tizen.ml.pipeline.registerCustomFilter("flattenFilter", filterCB,
                                                  inputTI, outputTI,
                                                  console.warn);

pipelineDefinition = "videotestsrc num-buffers=3 " +
                    "! video/x-raw,width=20,height=15,format=BGRA " +
                    "! tensor_converter " +
                    "! tensor_filter framework=custom-easy model=flattenFilter "
                    + "! fakesink";
pipeline = tizen.ml.pipeline.createPipeline(pipelineDefinition);
pipeline.start();

// hello
// WebAPIException {name: "AbortError", message:
// "CustomFilter has thrown exception: InvalidStateErr CustomFilter has
// thrown exception: InvalidStateError: The custom filter is processing
// data now. Stop the pipeline to unregister the filter."}

// <no deadlock>

///////////////////////////////////////////////////////////////////////////
inputTI = new tizen.ml.TensorsInfo();
inputTI.addTensorInfo("3D", "UINT8", [4, 20, 15, 1]);
outputTI = new tizen.ml.TensorsInfo();
outputTI.addTensorInfo("ti1", "UINT8", [1200]);

customFilter = function (input, output) {
  try {
      inputTI.dispose();
      outputTI.dispose();
      pipeline.stop();
      pipeline.dispose();
  } catch (err) {
      console.warn(err);
  }
}

tizen.ml.pipeline.registerCustomFilter("flattenFilter", customFilter, inputTI, outputTI);

pipelineDefinition = "videotestsrc num-buffers=3 " +
                    "! video/x-raw,width=20,height=15,format=BGRA " +
                    "! tensor_converter " +
                    "! tensor_filter framework=custom-easy model=flattenFilter " +
                    "! fakesink";
pipeline = tizen.ml.pipeline.createPipeline(pipelineDefinition);
pipeline.start();

// WebAPIException {name: "InvalidStateError",
// message: "Pipeline cannot be disposed when at least one custom filter
// is currently processing data.",

// <no deadlock>

///////////////////////////////////////////////////////////////////////////
// Valid CustomFilter callback - the happy scenario
var inputTI = new tizen.ml.TensorsInfo();
inputTI.addTensorInfo('ti1', 'UINT8', [4, 20, 15, 1]);
var outputTI = new tizen.ml.TensorsInfo();
outputTI.addTensorInfo('ti1', 'UINT8', [1200]);
var flattenAndSet123 = function(input, output) {
    console.log("Custom filter called");

    var rawOutputData = new Uint8Array(1200);
    for (var i = 0; i < rawOutputData.length; ++i) {
        rawOutputData[i] = 123;
    }

    output.setTensorRawData(0, rawOutputData);
    return 0;
}

tizen.ml.pipeline.registerCustomFilter('testfilter2', flattenAndSet123, inputTI,
    outputTI, function errorCallback(error) {
        console.warn('custom filter error:') ; console.warn(error);
    });

var pipeline_def = "videotestsrc num-buffers=3 "
                   + "! video/x-raw,width=20,height=15,format=BGRA "
                   + "! tensor_converter "
                   + "! tensor_filter framework=custom-easy model=testfilter2 "
                   + "! appsink name=mysink";

var pipeline = tizen.ml.pipeline.createPipeline(pipeline_def,
                                                state => {console.log(state);})

pipeline.registerSinkListener('mysink', function(sinkName, data) {
    console.log('SinkListener for "' + sinkName + '" sink called');
    console.log(data);
})

// READY
// Custom filter called
// PAUSED

pipeline.start()

// PLAYING
// <CustomFilter and SinkListener callbacks' outputs 3 times>

////////////////////////////////////////////////////////////

// Valid CustomFilter callback - the happy scenario; ignore the data

var inputTI = new tizen.ml.TensorsInfo();
inputTI.addTensorInfo('ti1', 'UINT8', [4, 20, 15, 1]);
var outputTI = new tizen.ml.TensorsInfo();
outputTI.addTensorInfo('ti1', 'UINT8', [1200]);
var flattenPlusOne = function(input, output) {
    console.log("Custom filter called");
        return 1; // ignore data
}

tizen.ml.pipeline.registerCustomFilter('testfilter2', flattenPlusOne, inputTI,
    outputTI, function errorCallback(error) {
        console.warn('custom filter error:') ; console.warn(error);
    });

var pipeline_def = "videotestsrc num-buffers=3 "
                   + "! video/x-raw,width=20,height=15,format=BGRA "
                   + "! tensor_converter "
                   + "! tensor_filter framework=custom-easy model=testfilter2 "
                   + "! appsink name=mysink";

var pipeline = tizen.ml.pipeline.createPipeline(pipeline_def,
                                                state => {console.log(state);})

pipeline.registerSinkListener('mysink', function(sinkName, data) {
    console.log('SinkListener for "' + sinkName + '" sink called');
    console.log(data);
})

// READY
// Custom filter called
// Custom filter called
// Custom filter called
// PAUSED

pipeline.start()

// PLAYING

////////////////////////////////////////////////////////////

// Valid CustomFilter callback - CustomFilter returns an error

var inputTI = new tizen.ml.TensorsInfo();
inputTI.addTensorInfo('ti1', 'UINT8', [4, 20, 15, 1]);
var outputTI = new tizen.ml.TensorsInfo();
outputTI.addTensorInfo('ti1', 'UINT8', [1200]);
var flattenPlusOne = function(input, output) {
    console.log("Custom filter called");
        return -1;
}

tizen.ml.pipeline.registerCustomFilter('testfilter2', flattenPlusOne, inputTI,
    outputTI, function errorCallback(error) {
        console.warn('custom filter error:') ; console.warn(error);
    });

var pipeline_def = "videotestsrc num-buffers=3 "
                   + "! video/x-raw,width=20,height=15,format=BGRA "
                   + "! tensor_converter "
                   + "! tensor_filter framework=custom-easy model=testfilter2 "
                   + "! appsink name=mysink";

var pipeline = tizen.ml.pipeline.createPipeline(pipeline_def,
                                                state => {console.log(state);})

pipeline.registerSinkListener('mysink', function(sinkName, data) {
    console.log('SinkListener for "' + sinkName + '" sink called');
    console.log(data);
})

// READY
// Custom filter called
// PAUSED

pipeline.start()

// PLAYING

////////////////////////////////////////////////////////////

// Invalid CustomFilterOutput.status
var inputTI = new tizen.ml.TensorsInfo();
inputTI.addTensorInfo('ti1', 'UINT8', [4, 20, 15, 1]);
var outputTI = new tizen.ml.TensorsInfo();
outputTI.addTensorInfo('ti1', 'UINT8', [1200]);
var flattenPlusOne = function(input) {
    console.log("Custom filter called");

    return 123;
}

tizen.ml.pipeline.registerCustomFilter('testfilter2', flattenPlusOne, inputTI,
    outputTI, function errorCallback(error) {
        console.warn('custom filter error:') ; console.warn(error);
    });

var pipeline_def = "videotestsrc num-buffers=3 "
                   + "! video/x-raw,width=20,height=15,format=BGRA "
                   + "! tensor_converter "
                   + "! tensor_filter framework=custom-easy model=testfilter2 "
                   + "! appsink name=mysink";

var pipeline = tizen.ml.pipeline.createPipeline(pipeline_def,
                                                state => {console.log(state);})

pipeline.registerSinkListener('mysink', function(sinkName, data) {
    console.log('SinkListener for "' + sinkName + '" sink called');
    console.log(data);
})

// InvalidValuesError,
//  message: "CustomFilterOutput.status === 1 is the only legal positive value"

////////////////////////////////////////////////////////////
// Check if {input, output}.dispose() and input.setTensorRawData()
// have any effect

var inputTI = new tizen.ml.TensorsInfo();
inputTI.addTensorInfo('ti1', 'UINT8', [4, 20, 15, 1]);
var outputTI = new tizen.ml.TensorsInfo();
outputTI.addTensorInfo('ti1', 'UINT8', [1200]);
var flattenAndSet123 = function(input, output) {
    console.log("Custom filter called");

    // dispose should have no efect
    input.dispose();
    console.log('input count: ' + input.tensorsInfo.count);
    // dispose should have no efect
    input.dispose();
    console.log('output count: ' + output.tensorsInfo.count);

    var rawOutputData = new Uint8Array(1200);
    for (var i = 0; i < rawOutputData.length; ++i) {
        rawOutputData[i] = 123;
    }

    output.setTensorRawData(0, rawOutputData);

    // this call should have no effect
    input.setTensorRawData(0, rawOutputData);

    return 0;
}

tizen.ml.pipeline.registerCustomFilter('testfilter2', flattenAndSet123, inputTI,
    outputTI, function errorCallback(error) {
        console.warn('custom filter error:') ; console.warn(error);
    });

var pipeline_def = "videotestsrc num-buffers=3 "
                   + "! video/x-raw,width=20,height=15,format=BGRA "
                   + "! tensor_converter "
                   + "! tensor_filter framework=custom-easy model=testfilter2 "
                   + "! appsink name=mysink";

var pipeline = tizen.ml.pipeline.createPipeline(pipeline_def,
                                                state => {console.log(state);})

pipeline.registerSinkListener('mysink', function(sinkName, data) {
    console.log('SinkListener for "' + sinkName + '" sink called');
    console.log(data);
})

Change-Id: I4c5b773203239321de5826b5e4908c50cd84b7d6
Signed-off-by: Pawel Wasowski <p.wasowski2@samsung.com>
---

diff --git a/src/ml/ml_pipeline_custom_filter.cc b/src/ml/ml_pipeline_custom_filter.cc
index 710481b3..017a0aab 100644
--- a/src/ml/ml_pipeline_custom_filter.cc
+++ b/src/ml/ml_pipeline_custom_filter.cc
@@ -135,7 +135,12 @@ CustomFilter::~CustomFilter() {
   ScopeLogger("name: [%s]_, listener_name_: [%s], custom_filter_: [%p]", name_.c_str(),
               listener_name_.c_str(), custom_filter_);
 
-  Unregister();
+  auto ret = Unregister();
+  if (!ret) {
+    std::lock_guard<std::mutex> lock{CustomFilter::valid_filters_mutex_};
+    CustomFilter::valid_filters_.erase(this);
+  }
+
   tensors_info_manager_ptr_->DisposeTensorsInfo(input_tensors_info_ptr_);
   tensors_info_manager_ptr_->DisposeTensorsInfo(output_tensors_info_ptr_);
 }
diff --git a/src/ml/ml_pipeline_manager.h b/src/ml/ml_pipeline_manager.h
index d39244f1..9e617b78 100644
--- a/src/ml/ml_pipeline_manager.h
+++ b/src/ml/ml_pipeline_manager.h
@@ -140,8 +140,16 @@ class PipelineManager {
   common::Instance* instance_ptr_;
   TensorsInfoManager* tensors_info_manager_;
   TensorsDataManager* tensors_data_manager_;
-  std::map<int, std::unique_ptr<Pipeline>> pipelines_;
+
+  /*
+   * Don't change the order of custom_fiters_ and pipelines_ - custom_filters_
+   * should be destroyed after pipelines_.
+   * This is because CustomFilters registered in a running Pipelines cannot
+   * be unregistered by native API. Pipelines have to be stopped and destroyed
+   * first.
+   */
   std::unordered_map<std::string, std::unique_ptr<CustomFilter>> custom_filters_;
+  std::map<int, std::unique_ptr<Pipeline>> pipelines_;
 };
 
 }  // namespace ml