-
-
Notifications
You must be signed in to change notification settings - Fork 101
lwipClient - fix mem pool leak #207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I'm not seeing it. Sure using a std::shared_ptr would be a pretty clean solution, but you could always just call delete (or mem_free) in the Destructor. Having a mem_alloc (a C function) inside of reset (a member of a C++ template class) just feels super weird. What do you think?
you could always just call delete (or mem_free) in the Destructor.
no. Client must be copyable. multiple copies of Client can point to the same TCP connection
having a mem_alloc (a C function) inside of reset (a member of a C++ template class) just feels super weird
not really. the resetfunction is just a assignment which allows to specify a deleter. uses of shared_ptr should be written as close as possible as with a basic pointer
aentinger
commented
Dec 14, 2023
Client must be copyable
Overload copy-ctor. But yeah, I do see your point somewhat. Can you prepare a adjacent PR showing how you'd like to take this further?
JAndrassy
commented
Dec 14, 2023
Can you prepare a adjacent PR showing how you'd like to take this further?
ok
f915d83 to
ebe2184
Compare
andreagilardoni
commented
Jan 3, 2024
This issue depends on arduino/ArduinoCore-API#218
This issue depends on arduino/ArduinoCore-API#218
no. tcp_client is not a class
and WiFiClient is never deleted from LwipClent pointer and it even doesn't make sense to do new for WiFiClient. that is why Arduino is not really missing the virtual destructor.
fix mem pool leak in lwipClient with shared_ptr with custom deleter.
for lwipClient created from lwipServer the deleter doesn't free the struct. server releases the clients it manages. it would be better if the server used an array of lwipClient objects. then the shared_ptr in them would hold the pointer. maybe in a follow up PR if this is merged.