!2378 bugfix for the concurrency competition between the reuse layer …

…and the creation layer

From: @taotao-sauce 
Reviewed-by: @wangfengtu, @xuxuepeng 
Signed-off-by: @xuxuepeng

CI/test_cases/image_cases/image_load.sh

@@ -79,8 +79,55 @@ function test_image_load()
   return ${ret}
 }
 
+function test_concurrent_load()
+{
+  local ret=0
+  local test="isula load image test => (${FUNCNAME[@]})"
+
+  msg_info "${test} starting..."
+
+  # clean exist image
+  ubuntu_id=`isula inspect -f '{{.image.id}}' ubuntu`
+  busybox_id=`isula inspect -f '{{.image.id}}' busybox`
+  isula rmi $ubuntu_id $busybox_id
+
+  concurrent_time=10
+  for i in `seq 1 $concurrent_time`
+  do
+      isula load -i $mult_image &
+      pids[$i]=$!
+  done
+
+  for i in `seq 1 $concurrent_time`;do
+      wait ${pids[$i]}
+      [[ $? -ne 0 ]] && msg_err "${FUNCNAME[0]}:${LINENO} - fail to do isulad load $i" && ((ret++))
+  done
+
+  ubuntu_id=`isula inspect -f '{{.image.id}}' ubuntu`
+  [[ $? -ne 0 ]] && msg_err "${FUNCNAME[0]}:${LINENO} - fail to inspect image: ubuntu" && ((ret++))
+
+  top_layer_id=$(isula inspect -f '{{.image.top_layer}}' ${ubuntu_id})
+
+  busybox_id=`isula inspect -f '{{.image.id}}' busybox`
+  [[ $? -ne 0 ]] && msg_err "${FUNCNAME[0]}:${LINENO} - fail to inspect image: busybox" && ((ret++))
+
+  # delete image after concurrent load
+  isula rmi $ubuntu_id $busybox_id
+  [[ $? -ne 0 ]] && msg_err "${FUNCNAME[0]}:${LINENO} - failed to remove image ${ubuntu_id} and ${busybox_id}" && ((ret++))
+
+  ls -l /var/lib/isulad/storage/overlay-layers
+  local top_layer_dir=/var/lib/isulad/storage/overlay-layers/${top_layer_id}
+  test -e ${top_layer_dir}
+  [[ $? -eq 0 ]] && msg_err "${FUNCNAME[0]}:${LINENO} - top layer dir ${top_layer_id} exist after delete image" && ((ret++))
+
+  msg_info "${test} finished with return ${ret}..."
+  return ${ret}
+}
+
 declare -i ans=0
 
+test_concurrent_load || ((ans++))
+
 test_image_load || ((ans++))
 
 show_result ${ans} "${curr_path}/${0}"

src/daemon/modules/image/oci/oci_load.c

@@ -680,6 +680,12 @@ static int oci_load_set_layers_info(load_image_t *im, const image_manifest_items
     char *parent_chain_id_sha256 = "";
     char *id = NULL;
     char *parent_chain_id = NULL;
+    // exist_flag is used to mark whether a non-existent layer has been encountered during this layer reuse process.
+    // 1.exist_flag is true if the layers are currently reusable;
+    // 2.exist_flag is false if encounter an uncreated layer that cannot be reused
+    // Prevent concurrent competition between the creation layer function
+    // and the reuse layer function on the im -> layer_of_hold_refs variable
+    bool exist_flag = true;
 
     if (im == NULL || manifest == NULL || dstdir == NULL) {
         ERROR("Invalid input params image or manifest is null");
@@ -761,7 +767,7 @@ static int oci_load_set_layers_info(load_image_t *im, const image_manifest_items
             goto out;
         }
 
-        if (storage_inc_hold_refs(id) == 0) {
+        if (exist_flag && storage_inc_hold_refs(id) == 0) {
             free(im->layer_of_hold_refs);
             im->layer_of_hold_refs = util_strdup_s(id);
             if (parent_chain_id != NULL && storage_dec_hold_refs(parent_chain_id) != 0) {
@@ -781,6 +787,7 @@ static int oci_load_set_layers_info(load_image_t *im, const image_manifest_items
             continue;
         }
 
+        exist_flag = false;
         if (check_and_set_digest_from_tarball(im->layers[i], conf->rootfs->diff_ids[i]) != 0) {
             ERROR("Check layer digest failed");
             ret = -1;

src/daemon/modules/image/oci/registry/registry.c

@@ -1516,6 +1516,12 @@ static int fetch_all(pull_descriptor *desc)
     struct layer_list *list = NULL;
     pthread_t tid = 0;
     struct timespec ts = { 0 };
+    // exist_flag is used to mark whether a non-existent layer has been encountered during this layer reuse process.
+    // 1.exist_flag is true if the layers are currently reusable;
+    // 2.exist_flag is false if encounter an uncreated layer that cannot be reused
+    // Prevent concurrent competition between the creation layer function
+    // and the reuse layer function on the im -> layer_of_hold_refs variable
+    bool exist_flag = true;
 
     if (desc == NULL) {
         ERROR("Invalid NULL param");
@@ -1547,7 +1553,7 @@ static int fetch_all(pull_descriptor *desc)
 
         // Skip layer that already exist in local store
         list = storage_layers_get_by_compress_digest(desc->layers[i].digest);
-        if (list != NULL) {
+        if (exist_flag && list != NULL) {
             for (j = 0; j < list->layers_len; j++) {
                 if ((list->layers[j]->parent == NULL && i == 0) ||
                     (parent_chain_id != NULL && list->layers[j]->parent != NULL &&
@@ -1579,6 +1585,7 @@ static int fetch_all(pull_descriptor *desc)
                 continue;
             }
         }
+        exist_flag = false;
 
         // parent_chain_id = NULL means no parent chain match from now on, so no longer need
         // to get layers by compressed digest to reuse layer.