Fix memory leak in failed DefaultCallbackProvider constructor #1200
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
.. is required for fetching files from parent dir ... without .. it throws error
Co-authored-by: David D <[email protected]>
* Fix typo in readme * Update Readme Debug section: add solution to cmake error "could not find JNI" Co-authored-by: Hassan Sahibzada <[email protected]>
Co-authored-by: Hassan Sahibzada <[email protected]>
Co-authored-by: Hassan Sahibzada <[email protected]>
Hoping this help others avoid hours of frustration. As noted in the helpful but well hidden issue comment awslabs#193 (comment), the Gstreamer examples for RTSP do not run at all. I'm no Gstreamer expert but using `h264parse` works perfectly, while `video/x-h264, format=avc,alignment=au` just causes the pipeline to hang.
Co-authored-by: David D <[email protected]>
* Update for develop (awslabs#740) * Minor syntax fix .. is required for fetching files from parent dir ... without .. it throws error * Update README and travis.yml to acknowledge develop * Fix typo in readme * add free missing (awslabs#746) Co-authored-by: David D <[email protected]> * Update readme (awslabs#744) * Fix typo in readme * Update Readme Debug section: add solution to cmake error "could not find JNI" Co-authored-by: Hassan Sahibzada <[email protected]> * Add instruction to set offline mode (awslabs#741) Co-authored-by: Hassan Sahibzada <[email protected]> Co-authored-by: Ketul shah <[email protected]> Co-authored-by: Jeremy Gunawan <[email protected]> Co-authored-by: waikup83 <[email protected]> Co-authored-by: David D <[email protected]> Co-authored-by: Hassan Sahibzada <[email protected]> Co-authored-by: Divya Sampath Kumar <[email protected]> * updated cmake verison from 2.8 to 3.6.3 * Fix bug that leads to losing initial frames * Update to top of producer C commit * Untie stream-name and thing-name (awslabs#785) * untie stream-name and thing-name * fixup spacing Co-authored-by: tom schuring <[email protected]> * update docs for iot-thing-name usage * Expose file logging jni develop (awslabs#770) * expose addFileLoggerPlatformCallbacksProvider in Java with JNI * updated addFileLoggerPlatformCallbacksProvider to parse file path * added a callback from JNI to a Java function for logging * updated logPrintFunc calback * missed client_handle initialization in constructor * added comments * white space change to trigger travis * Images feature and sample support * GitHub actions setup (awslabs#815) * github actions setup; disable travis for develop * fix syntax error: * fix syntax error * fix linux builds * fix linux builds * fix linux builds * add missing packages * add keys to env vars * check env vars * check env vars * fix builds * fix builds on ga * test mac clang on ga * test mac clang on ga * test mac clang on ga * test mac clang on ga * test mac clang on ga * unsetting the token * unsetting the token * test macos clang * test macos clang * stop travis builds on each push * use oidc * fix windows build * fix test instruction * comment failing builds on travis and ga * update gcc and macos versions * update cmake * update cmake * use latest os, update log levels, uncomment travis * update failing build * update failing build * trigger travis * trigger travis * Revert "trigger travis" This reverts commit d511f15. * update msvc path, add windows to ga, remove from travis * trigger travis * trigger travis w/o passing builds * add cpath and ldflags for mac build * Revert "add cpath and ldflags for mac build" This reverts commit fafad7b. * test ubsan build * fix cmake instruction for mac-gcc * remove travis file * remove travis checks * Update Producer-c to new master * Updated to the ACTUAL master... * Fix OIDC for GitHub Actions (awslabs#824) * use oidc right before tests * run ga builds for branch * fix test path * update branches * set creds expiration time * testing re-order tags changes * Updated Producer, now putEventMetadata returns with an error if the first cluster has not been started * updated kvs_gstreamer_sample.cpp to also use the event metadata feature on every key frame * Add ARM build to GA, log4cplus host name fix for cross compilation (awslabs#831) * setup arm cross compilation * update the package list * fix in * specify host in log4cplus * specify host in log4cplus * specify host in log4cplus * run all builds with new host config in log4cplus * build with gstreamer and dependencies * build with gstreamer and dependencies * build with gstreamer and dependencies and openssl * build with gstreamer and openssl * specify build and host * specify build and host * specify build host and target * specify same build and host * specify build and host in all cmake files * use a different compiler * use a different compiler * echo compiler * set CC and CXX correctly * set CC and CXX correctly * set openssl platform * set openssl platform * set openssl platform * expose host name for log4cplus * change log4cplus host name * fix cmake syntax error * fix cmke log4plus condition * fix cmake log4plus condition * fix cmake log4plus condition * change var name cmake * change var name in ci * update cmake instruction in ci * check conditions for log4cplus in cmake * check conditions for log4cplus in cmake * use set instead of option in cmake * check messages * fix ci cmake instruction * fix ci cmake instruction * add definition * use env var * use env var * use build args * update readme * remove host-name from qemu arm build * update readme for cross-compile instructions * add aarch64; move log config file * remove qemu * fix config file path (awslabs#835) * Duplicate fix, sample cleanup * Test was 'sometimes' triggering. Needed more frames to ensure its success Co-authored-by: Ketul shah <[email protected]> Co-authored-by: Jeremy Gunawan <[email protected]> Co-authored-by: waikup83 <[email protected]> Co-authored-by: David D <[email protected]> Co-authored-by: Hassan Sahibzada <[email protected]> Co-authored-by: Divya Sampath Kumar <[email protected]> Co-authored-by: Niyati Maheshwari <[email protected]> Co-authored-by: clogwog <[email protected]> Co-authored-by: tom schuring <[email protected]>
* expose streaming flags in java with jni (awslabs#849) * expose streaming flags in java with jni * fix indentation * Elaborated on how to work with IoT credentials. (awslabs#851) It is not a straight-forward process to enable KVS and IoT credentials. It is required to read and follow through https://docs.aws.amazon.com/kinesisvideostreams/latest/dg/how-iot.html. * Add storePressurePolicy call to JNI (awslabs#859) * add store pressure policy * update ci to add update * set flag in pClientInfo (awslabs#863) Co-authored-by: Niyati Maheshwari <[email protected]> Co-authored-by: Byong-Wu Chong <[email protected]>
Signed-off-by: Alex Li <[email protected]> Signed-off-by: Alex Li <[email protected]>
see awslabs#878 for more information related to this update
* Update for develop (awslabs#740) * Minor syntax fix .. is required for fetching files from parent dir ... without .. it throws error * Update README and travis.yml to acknowledge develop * Fix typo in readme * add free missing (awslabs#746) Co-authored-by: David D <[email protected]> * Update readme (awslabs#744) * Fix typo in readme * Update Readme Debug section: add solution to cmake error "could not find JNI" Co-authored-by: Hassan Sahibzada <[email protected]> * Add instruction to set offline mode (awslabs#741) Co-authored-by: Hassan Sahibzada <[email protected]> Co-authored-by: Ketul shah <[email protected]> Co-authored-by: Jeremy Gunawan <[email protected]> Co-authored-by: waikup83 <[email protected]> Co-authored-by: David D <[email protected]> Co-authored-by: Hassan Sahibzada <[email protected]> Co-authored-by: Divya Sampath Kumar <[email protected]> * updated cmake verison from 2.8 to 3.6.3 * Fix bug that leads to losing initial frames * Update to top of producer C commit * Untie stream-name and thing-name (awslabs#785) * untie stream-name and thing-name * fixup spacing Co-authored-by: tom schuring <[email protected]> * update docs for iot-thing-name usage * Expose file logging jni develop (awslabs#770) * expose addFileLoggerPlatformCallbacksProvider in Java with JNI * updated addFileLoggerPlatformCallbacksProvider to parse file path * added a callback from JNI to a Java function for logging * updated logPrintFunc calback * missed client_handle initialization in constructor * added comments * white space change to trigger travis * GitHub actions setup (awslabs#815) * github actions setup; disable travis for develop * fix syntax error: * fix syntax error * fix linux builds * fix linux builds * fix linux builds * add missing packages * add keys to env vars * check env vars * check env vars * fix builds * fix builds on ga * test mac clang on ga * test mac clang on ga * test mac clang on ga * test mac clang on ga * test mac clang on ga * unsetting the token * unsetting the token * test macos clang * test macos clang * stop travis builds on each push * use oidc * fix windows build * fix test instruction * comment failing builds on travis and ga * update gcc and macos versions * update cmake * update cmake * use latest os, update log levels, uncomment travis * update failing build * update failing build * trigger travis * trigger travis * Revert "trigger travis" This reverts commit d511f15. * update msvc path, add windows to ga, remove from travis * trigger travis * trigger travis w/o passing builds * add cpath and ldflags for mac build * Revert "add cpath and ldflags for mac build" This reverts commit fafad7b. * test ubsan build * fix cmake instruction for mac-gcc * remove travis file * remove travis checks * Fix OIDC for GitHub Actions (awslabs#824) * use oidc right before tests * run ga builds for branch * fix test path * update branches * set creds expiration time * Add ARM build to GA, log4cplus host name fix for cross compilation (awslabs#831) * setup arm cross compilation * update the package list * fix in * specify host in log4cplus * specify host in log4cplus * specify host in log4cplus * run all builds with new host config in log4cplus * build with gstreamer and dependencies * build with gstreamer and dependencies * build with gstreamer and dependencies and openssl * build with gstreamer and openssl * specify build and host * specify build and host * specify build host and target * specify same build and host * specify build and host in all cmake files * use a different compiler * use a different compiler * echo compiler * set CC and CXX correctly * set CC and CXX correctly * set openssl platform * set openssl platform * set openssl platform * expose host name for log4cplus * change log4cplus host name * fix cmake syntax error * fix cmke log4plus condition * fix cmake log4plus condition * fix cmake log4plus condition * change var name cmake * change var name in ci * update cmake instruction in ci * check conditions for log4cplus in cmake * check conditions for log4cplus in cmake * use set instead of option in cmake * check messages * fix ci cmake instruction * fix ci cmake instruction * add definition * use env var * use env var * use build args * update readme * remove host-name from qemu arm build * update readme for cross-compile instructions * add aarch64; move log config file * remove qemu * fix config file path (awslabs#835) * expose streaming flags in java with jni (awslabs#849) * expose streaming flags in java with jni * fix indentation * Elaborated on how to work with IoT credentials. (awslabs#851) It is not a straight-forward process to enable KVS and IoT credentials. It is required to read and follow through https://docs.aws.amazon.com/kinesisvideostreams/latest/dg/how-iot.html. * Add storePressurePolicy call to JNI (awslabs#859) * add store pressure policy * update ci to add update * set flag in pClientInfo (awslabs#863) * Allow using CPP SDK as a dependency (awslabs#905) * Allow using CPP SDK as a dependency * Update README * Fix static build (awslabs#910) * Add static build to CI * gstkvssink should have static linkage in a static build * log4cplus should be a static lib in a static build * Don't force full static link * Update libkvscproducer-CMakeLists.txt Update Producer C Commit hash Co-authored-by: Hassan Sahibzada <[email protected]> * create new sample that uses kvssink and increase timeout for CI tests (awslabs#917) * Fix GHA CI (awslabs#944) * fix failing builds + upgrade gtest to 1.12 * install missing package * change os and gcc versions * exclude windows build * use event triggers * Fix CI for forked branches (awslabs#946) * fix ci for fork * blank space change to trigger CI * Add support for milliseconds in the file upload sample (awslabs#947) * Minor syntax fix .. is required for fetching files from parent dir ... without .. it throws error * Fix typo in readme * add free missing (awslabs#746) Co-authored-by: David D <[email protected]> * Update readme (awslabs#744) * Fix typo in readme * Update Readme Debug section: add solution to cmake error "could not find JNI" Co-authored-by: Hassan Sahibzada <[email protected]> * Add instruction to set offline mode (awslabs#741) Co-authored-by: Hassan Sahibzada <[email protected]> * Update README and travis.yml to acknowledge develop (awslabs#739) Co-authored-by: Hassan Sahibzada <[email protected]> * Gstreamer RTSP Docs Update (awslabs#712) Hoping this help others avoid hours of frustration. As noted in the helpful but well hidden issue comment awslabs#193 (comment), the Gstreamer examples for RTSP do not run at all. I'm no Gstreamer expert but using `h264parse` works perfectly, while `video/x-h264, format=avc,alignment=au` just causes the pipeline to hang. * free missing property (awslabs#767) Co-authored-by: David D <[email protected]> * Update gstkvssink.cpp * Added millisecond support to kvs_gstreamer_file_upload_sample + gstkvssink --------- Co-authored-by: Ketul shah <[email protected]> Co-authored-by: waikup83 <[email protected]> Co-authored-by: David D <[email protected]> Co-authored-by: Hassan Sahibzada <[email protected]> Co-authored-by: Divya Sampath Kumar <[email protected]> Co-authored-by: jdelapla <[email protected]> Co-authored-by: Anton Vattay <[email protected]> Co-authored-by: bkneff <[email protected]> * Add gstreamer debug instructions for mac (awslabs#948) Co-authored-by: Divya Sampath Kumar <[email protected]> * Pass parameter by reference to avoid copy constructor (awslabs#949) * pass parameter by reference to avoid copy constructor * nit style --------- Co-authored-by: James Delaplane <[email protected]> * Update openssl version (awslabs#950) * Replace pull_request_target with pull_request (awslabs#955) * replace pull_request_target with pull_request; update versions for codeql * add develop to codeql * Kvssink canary producer cpp (awslabs#984) * Cmake changes after release 3.3.1 * Added fragment ack handler, reverting changes related to edge * Added gsignal element for fragment ack * Fixed cmake dependency for canarydemos * Reverted c++ versrion to 11 * Commented out g signal in fragment act handler * Re-Added the g signal * Kvs sink initialized to avoid bad memory access * Intialized custom objet kvssink * Moved part of cloud watch metrics and logs * Resolved constructor issue * Added remaining functions for cloud watch logs * Reverted changes for cloudwatch shift * Reverted Cmake changes * Reverted remaining changes * Added stream metric signal * G signal changes * G signal parameter changes * Removed persisted ack check * Changed return type in g signal * Added custome object for put frame * Metric struct update * Intialized custome object metrics * Added check for key frame * Added client metrics element * Name changed * Added first frame emit * First frame changes * Reverted first frame changes * Left change * Added g signal emit on first successfull frame * Fixed compile error * Startup latency test_1 * Added first frame check * Resolved comments on PR 984 * Resolved comments on PR 984-remaining * Resolved comments in PR 984_03-16 * resolved comment in PR-276_03-21 * Uncomment windows (awslabs#990) * Signals in develop (awslabs#957) * Signals in develop * Add signals and static analysis fixes * Signals in develop * Add better logging * Pull in new producer C commit * Update gstkvssink.cpp * use get metric property * Use log and throw and catch --------- Co-authored-by: NikunjSarda <[email protected]> * Timeouts setup (awslabs#992) * Adding timeouts configurability * Add jni for new timeouts and create stream flag * Revert log4cplus version and cxx standard * Revert versions * Revert C++ version to 11 * fix warnings (awslabs#994) * Disable caching by default (awslabs#995) * Update CMakeLists.txt (awslabs#997) * Update CMakeLists.txt when building shared the kvs_gstreamer_sample.cpp example fails LINKING fix awslabs#930 * Additional CI Build --------- Co-authored-by: tro <[email protected]> * fix timestamp bug (awslabs#999) * Enable caching by default (awslabs#1020) * Invoke stopSync in NULL state transition (awslabs#1023) * Add EOS specific handling * New logging * New logging 1 * New logging 2 * New logging 4 * New logging 5 * New logging 5 * Check with bool * listen on NULL state transition * Version user agent string from kvssink (awslabs#1033) * Use permalinks in buffering.md (awslabs#1038) * fix ci * resolve missed git conflicts * remove duplicate decl * update producer c * Auth segfault fix (awslabs#1046) * Auth segfault fix * Added unit tests * Update raspberry-pi.md (awslabs#1048) * Update raspberry-pi.md * Update raspberry-pi.md --------- Co-authored-by: Josh McMenemy <[email protected]> * fix(kvs/sink_gstreamer_sample): Use videotestsrc and add additional debug logging (awslabs#1047) * Switch from autovideosrc to videotestsrc and add additional debug logging * Add null check for ksvideosrc and adjust comments * Revise logging again * Exit early if one of the elements cant be created, log which one * Check return value of gstreamer_init * Update commit to use release 1.5.0 of producer SDK --------- Co-authored-by: jdelapla <[email protected]> Co-authored-by: Ketul shah <[email protected]> Co-authored-by: Jeremy Gunawan <[email protected]> Co-authored-by: waikup83 <[email protected]> Co-authored-by: David D <[email protected]> Co-authored-by: Divya Sampath Kumar <[email protected]> Co-authored-by: Niyati Maheshwari <[email protected]> Co-authored-by: clogwog <[email protected]> Co-authored-by: tom schuring <[email protected]> Co-authored-by: Byong-Wu Chong <[email protected]> Co-authored-by: Greg Breen <[email protected]> Co-authored-by: Benjamin Kim <[email protected]> Co-authored-by: Anton Vattay <[email protected]> Co-authored-by: bkneff <[email protected]> Co-authored-by: NikunjSarda <[email protected]> Co-authored-by: Jeremy Gunawan <[email protected]> Co-authored-by: tro <[email protected]> Co-authored-by: Yuma Mihira <[email protected]> Co-authored-by: Josh McMenemy <[email protected]>
* Update version * Update release tag
* update build-tag link * fix broken log-level link
* Create playback-issue.md * Fix spelling * Fix bulletpoint
* Set -DPKG_CONFIG_EXECUTABLE in windows bat * Update GStreamer version * Update gst pkgconfig path with msvc_x86_64 * Set to MacOS 12
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description of changes:
I noticed a leak in memory in our application and was able to trace it to the KVS C++ Producer Library.
This situation was mainly occuring when the producer could not reach the internet.
Our application is long-living, and will restart the pipeline with kvssink if it fails very quickly, so in times of internet outage we cycle through instances of kvssink fairly quickly.
As is visible in the graph, at the time this dump was taken, KVS Producer was responsible for 30GB of memory coming from createCurlApiCallbacks. On investigation, I was able trace this back to the DefaultCallbackProvider not freeing memory allocated to PClientCallbacks instance when the constructor failed.
This issue was fixed by transitioning the PClientCallbacks instance to a separate RAII instance that will automatically free when the DefaultCallbackProvider instance goes out of scope.
This change has been running in a test environment with the triggering conditions for 4 days and we have seen the leak resolved.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.