Fix geyser plugin unload crash #34026
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #34026 +/- ##
=======================================
Coverage 81.9% 81.9%
=======================================
Files 811 811
Lines 219424 219428 +4
=======================================
+ Hits 179761 179770 +9
+ Misses 39663 39658 -5 |
| current_plugin.on_unload(); | ||
| drop(current_plugin); | ||
| drop(current_lib); |
There was a problem hiding this comment.
Just to make sure my understanding is correct, would it also resolve the crash if this line weren't here?
There was a problem hiding this comment.
Yes, according to https://doc.rust-lang.org/reference/destructors.html#drop-scopes. The variables/temporaries are dropped in the reverse order of the declaration or creation. I intentionally put these explicit drop here to ensure the plugin is dropped before the library.
There was a problem hiding this comment.
Right, but the library would be dropped automatically at the end of the block.
Anyway, I don't mind the verbosity.
There was a problem hiding this comment.
there should probably be a comment in the code that this is intentional
| current_plugin.on_unload(); | ||
| drop(current_plugin); | ||
| drop(current_lib); |
There was a problem hiding this comment.
Right, but the library would be dropped automatically at the end of the block.
Anyway, I don't mind the verbosity.
Problem
Unloading a simple plugin which does nothing other than creating the GeyserPlugin object is found to be crashing the validator. The reason for the crash is there are two objects when _drop_plugin is called: the Library object and the Box object. The current implementation drops the library first which already de-allocate the memory under the boxed GeyserPlugin. And when the GeyserPlugin is dropped, it tries to deallocate the memory and cause a segmentation fault.
Summary of Changes
Explicit ordering of the drop of Library and Box: drop the latter explicitly first.
Fixes #