fix(libsinsp): do not postpone observing fd erasing

[jasondellaluce]

What type of PR is this?

/kind bug

/kind cleanup

Any specific area of the project related to this PR?

/area libsinsp

Does this PR require a change in the driver versions?

What this PR does / why we need it:

Experimenting with the latest libs, I found that in some corner cases the sinsp observer callback on fd erasing can lead to use-after-free issues. This happens in cases where thread removal is less predictable, such as when main thread removal is deferred.

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

[github-actions]

Perf diff from master - unit tests

     3.10%     -0.76%  [.] sinsp_parser::process_event
     8.22%     +0.64%  [.] sinsp_evt::get_type
     1.76%     +0.54%  [.] next
    10.54%     -0.43%  [.] sinsp_parser::reset
     1.03%     -0.40%  [.] sinsp_parser::parse_context_switch
     3.75%     +0.38%  [.] gzfile_read
     1.88%     -0.34%  [.] std::_Hashtable<unsigned long, std::pair<unsigned long const, std::shared_ptr<ppm_evt_hdr> >, std::allocator<std::pair<unsigned long const, std::shared_ptr<ppm_evt_hdr> > >, std::__detail::_Select1st, std::equal_to<unsigned long>, std::hash<unsigned long>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_find_before_node
     5.47%     +0.27%  [.] next_event_from_file
     1.77%     +0.27%  [.] std::_Hashtable<long, std::pair<long const, std::shared_ptr<sinsp_threadinfo> >, std::allocator<std::pair<long const, std::shared_ptr<sinsp_threadinfo> > >, std::__detail::_Select1st, std::equal_to<long>, std::hash<long>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_find_before_node
     2.02%     +0.27%  [.] is_conversion_needed

Heap diff from master - unit tests

peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Heap diff from master - scap file

peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Benchmarks diff from master

Comparing gbench_data.json to /root/actions-runner/_work/libs/libs/build/gbench_data.json
Benchmark                                                         Time             CPU      Time Old      Time New       CPU Old       CPU New
----------------------------------------------------------------------------------------------------------------------------------------------
BM_sinsp_split_mean                                            +0.0252         +0.0251           148           151           147           151
BM_sinsp_split_median                                          +0.0304         +0.0303           147           152           147           152
BM_sinsp_split_stddev                                          +0.7548         +0.7551             1             1             1             1
BM_sinsp_split_cv                                              +0.7116         +0.7121             0             0             0             0
BM_sinsp_concatenate_paths_relative_path_mean                  -0.0302         -0.0303            62            60            62            60
BM_sinsp_concatenate_paths_relative_path_median                -0.0315         -0.0316            62            60            62            60
BM_sinsp_concatenate_paths_relative_path_stddev                +2.3597         +2.3558             0             0             0             0
BM_sinsp_concatenate_paths_relative_path_cv                    +2.4642         +2.4605             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_mean                     +0.0508         +0.0507            24            25            24            25
BM_sinsp_concatenate_paths_empty_path_median                   +0.0475         +0.0474            24            25            24            25
BM_sinsp_concatenate_paths_empty_path_stddev                   +2.5366         +2.5375             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_cv                       +2.3657         +2.3668             0             0             0             0
BM_sinsp_concatenate_paths_absolute_path_mean                  -0.1080         -0.1081            66            59            66            59
BM_sinsp_concatenate_paths_absolute_path_median                -0.1305         -0.1306            66            57            66            57
BM_sinsp_concatenate_paths_absolute_path_stddev                +4.3495         +4.3503             0             3             0             3
BM_sinsp_concatenate_paths_absolute_path_cv                    +4.9971         +4.9986             0             0             0             0
BM_sinsp_split_container_image_mean                            +0.0116         +0.0115           392           397           392           397
BM_sinsp_split_container_image_median                          +0.0110         +0.0108           393           397           393           397
BM_sinsp_split_container_image_stddev                          -0.1293         -0.1297             3             3             3             3
BM_sinsp_split_container_image_cv                              -0.1393         -0.1396             0             0             0             0

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.34%. Comparing base (ed95fff) to head (499b95d).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2260      +/-   ##
==========================================
- Coverage   75.34%   75.34%   -0.01%     
==========================================
  Files         279      279              
  Lines       34377    34368       -9     
  Branches     5872     5873       +1     
==========================================
- Hits        25903    25893      -10     
- Misses       8474     8475       +1     

Flag	Coverage Δ
libsinsp	75.34% <100.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Andreagit97]

/approve

[poiana]

LGTM label has been added.

Git tree hash: e0e0047b5fa6024ef3b5f3923de43336016ed225

[FedeDP]

/approve

[poiana]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97, FedeDP, jasondellaluce

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [Andreagit97,FedeDP,jasondellaluce]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[FedeDP]

Thanks for the fix @jasondellaluce !
/milestone 0.21.0