update(userspace): minor improvements on the plugin framework #1100
Conversation
Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
poiana
added
release-note
kind/bug
|
/milestone 0.11.0 |
| if (cur_async_handler) | ||
| { | ||
| delete cur_async_handler; | ||
| m_async_evt_handler.store(nullptr); |
There was a problem hiding this comment.
Shouldn't we first store nullptr (so that others threads loading it will get nullptr) and then delete the pointer?
Otherwise another thread could load the handler after we removed it but before we stored nullptr.
There was a problem hiding this comment.
Since this happens in the destructor of the plugin, we'd be in much trouble if another thread from the plugin code is still running. Also because we'd already unloaded the dynamic library. However, event though this is an unreachable case the suggestion is right and I applied just in case ❤️
|
LGTM label has been added. Git tree hash: 8ea56f56f727f7ea0368bd5e548637b702967c96
|
Co-authored-by: Federico Di Pierro <nierro92@gmail.com> Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
|
LGTM label has been added. Git tree hash: d387d117c61a0ae01be48c89e5bcbc459805e878
|
|
[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:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind bug
/kind cleanup
/kind design
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:
This polishes some inconsistencies in the plugin API and solves a data race in the async event handler of libsinsp.
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?: