[CLIENT-2887] Fix aerospike.Client() segfaulting when it fails to connect #608
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
How seg fault occurs
When
aerospike.Client(config)
is called, where config has a host / port that fails to connect:tp_new
to create a newaerospike.Client
object in Python. This presumably creates a new reference to theaerospike.Client
objectAerospikeClient_Type_Init()
(tp_init
) initializes theaerospike.Client
object in Python, populates a C client config struct using the Python client'sconfig
dictionary, then creates a new C client usingself->as = aerospike_new(&config);
. Fail to connect, so destroy C client usingaerospike_destroy(self->as);
aerospike.Client
object exists anymore, soAerospikeClient_Type_Dealloc()
(tp_dealloc
) is calledAerospikeClient_Type_Dealloc()
callsaerospike_destroy()
again on the same C client, causing a double free (seg fault)Fix
In
AerospikeClient_Type_Init()
, if C client fails to connect, don't destroy it and letAerospikeClient_Type_Dealloc()
handle it later.Implications of this fix
aerospike.client(config)
code must be changed to avoid a memory leakAerospikeClient_New()
is the Python client C code foraerospike.client(config)
. This is the main way to create a client, rather thanaerospike.Client(config)
.aerospike.client(config)
calls in this order:AerospikeClient_Type.tp_new()
to create theaerospike.Client
objectAerospikeClient_Type.tp_init()
to initialize theaerospike.Client
object.tp_init()
fails (i.e fails to connect),AerospikeClient_Type.tp_free()
destroys theaerospike.Client
object.After this PR's changes, when the client fails to connect with
aerospike.client(config)
,AerospikeClient_New()
must find another way to destroy the C client sincetp_init
no longer does it.tp_dealloc
destroys both the C client and Python client, so we replacetp_free()
with that.Valgrind
Dev: https://github.com/aerospike/aerospike-client-python/actions/runs/8914308860/job/24481631213
This branch: https://github.com/aerospike/aerospike-client-python/actions/runs/8913166351/job/24478118908
No new memory errors and there are less memory leaks in this branch
Build wheels all passes except for noise: https://github.com/aerospike/aerospike-client-python/actions/runs/9008696941