-
Notifications
You must be signed in to change notification settings - Fork 6k
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
[qt5cpp] delete callback data allocated before signal emission #7840
[qt5cpp] delete callback data allocated before signal emission #7840
Conversation
@@ -21,6 +22,19 @@ namespace {{this}} { | |||
QString stringValue(qint64 value); | |||
QString stringValue(bool value); | |||
|
|||
template <typename ObjPtrT> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe move this class to its own file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to suggest a more appropriate class name also?
I don't know if ObjLaterDeleter is appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I came up with object wrapper which is what it is..., :)
…lback_data # Conflicts: # modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/Qt5CPPGenerator.java # samples/client/petstore/qt5cpp/PetStore/PetStore.pro
{{/cppNamespaceDeclarations}} | ||
|
||
template <typename ObjectPtrT> | ||
class QObjectWrapper : public QObject { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also add the prefix to this class name as well
PR checklist
./bin/
to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.sh
and./bin/security/{LANG}-petstore.sh
if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\
.3.0.0
branch for changes related to OpenAPI spec 3.0. Default:master
.Description of the PR
Before emitting the signals declared in the API class when the callback from HTTP Worker object is received, there is memory allocation being made which is unknown to user code.
This PR adds a wrapper to any memory allocation being done and schedule deleteLater so that once the signal callback has been invoked, the object can be scheduled to be deleted.
Ideal case would be all memory allocation done by the generated code will be deallocated inside it, and user allocated memory will be done on user code.
Tests
There is a limitation with the current PetStore tests because when the callback is received the loop is exited which prevents deleteLater from running. So the PetStore still have the memory Leak visible.
@stkrwork @fvarose @ravinikam