cinemast / libjson-rpc-cpp

C++ framework for json-rpc (json remote procedure call)
MIT License
952 stars 316 forks source link

fixed fd leak #302

Closed skyfireitdiy closed 3 years ago

laudrup commented 3 years ago

While this most likely fixes the leaks, it still requires to manually call closesocket the right places which is quite error prone.

Instead, I'd suggest creating a simple RAII wrapper around the file descriptor that closes the file descriptor in the destructor.

If C++11 is required anyway, this can simply be std::unique_ptr with a custom deleter. Let me know if you need an example of how this could be done (with or without C++11).

Note that I'm not in any way a maintainer of this library, just wanted to share my thoughts.

skyfireitdiy commented 3 years ago

感谢你的回复,目前我只是从我们目前项目来考虑,做最简单的修改。 如果使用unique_ptr在控制对象的生命周期确实会更好一些。

Thank you for your reply. At present, I just consider our current project and make the simplest modification. It would be better if you use unique_ptr to control the life cycle of the object.

Get Outlook for Androidhttps://aka.ms/AAb9ysg


From: Kasper Laudrup @.> Sent: Wednesday, May 19, 2021 3:59:51 PM To: cinemast/libjson-rpc-cpp @.> Cc: 王茂斌 @.>; Author @.> Subject: Re: [cinemast/libjson-rpc-cpp] fixed fd leak (#302)

While this most likely fixes the leaks, it still requires to manually call closesocket the right places which is quite error prone.

Instead, I'd suggest creating a simple RAII wrapper around the file descriptor that closes the file descriptor in the destructor.

If C++11 is required anyway, this can simply be std::unique_ptr with a custom deleter. Let me know if you need an example of how this could be done (with or without C++11).

Note that I'm not in any way a maintainer of this library, just wanted to share my thoughts.

― You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/cinemast/libjson-rpc-cpp/pull/302#issuecomment-843848182, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABMIXAFC7QDOIMEHGQWRKHLTONVXPANCNFSM45D6KGTQ.

skyfireitdiy commented 3 years ago

While this most likely fixes the leaks, it still requires to manually call closesocket the right places which is quite error prone.

Instead, I'd suggest creating a simple RAII wrapper around the file descriptor that closes the file descriptor in the destructor.

If C++11 is required anyway, this can simply be std::unique_ptr with a custom deleter. Let me know if you need an example of how this could be done (with or without C++11).

Note that I'm not in any way a maintainer of this library, just wanted to share my thoughts.

I'm too busy these days, would you like to take the time to fix this fd leak?

cinemast commented 3 years ago

@laudrup I totally agree. The project requires some love in general. I will keep it in mind, thanks!

@skyfireitdiy thanks for opening the PRs.