Skip to content

Commit 7a9834d

Browse files
sirknightjsirknightj
committed
Fix kvssink memory leak & add CI
1 parent 0e542c1 commit 7a9834d

File tree

5 files changed

+196
-40
lines changed

5 files changed

+196
-40
lines changed

‎.github/configs/no_logs_configuration‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
log4cplus.rootLogger=OFF

‎.github/valgrind/glib.supp‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
{
2+
glib-dl-memory-leak-ubuntu-22.04
3+
Memcheck:Leak
4+
match-leak-kinds: definite
5+
fun:malloc
6+
fun:g_malloc
7+
obj:/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.4
8+
obj:/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.4
9+
fun:call_init.part.0
10+
fun:call_init
11+
fun:_dl_init
12+
obj:/usr/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2
13+
obj:*
14+
obj:*
15+
obj:*
16+
obj:*
17+
obj:*
18+
}
19+
20+
{
21+
glib-dl-memory-leak-ubuntu-20.04
22+
Memcheck:Leak
23+
match-leak-kinds: definite
24+
fun:malloc
25+
fun:g_malloc
26+
obj:/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.6400.6
27+
fun:call_init.part.0
28+
fun:call_init
29+
fun:_dl_init
30+
obj:/usr/lib/x86_64-linux-gnu/ld-2.31.so
31+
obj:*
32+
obj:*
33+
obj:*
34+
obj:*
35+
obj:*
36+
obj:*
37+
}

‎.github/workflows/kvssink.yml‎

Lines changed: 149 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -19,22 +19,23 @@ jobs:
1919
contents: read
2020
strategy:
2121
matrix:
22-
include:
22+
run-config:
2323
- os: Ubuntu 22.04
2424
image: public.ecr.aws/ubuntu/ubuntu:22.04_stable
2525
- os: Ubuntu 20.04
2626
image: public.ecr.aws/ubuntu/ubuntu:20.04_stable
2727
fail-fast: false
2828

29-
name: kvssink unit tests on ${{ matrix.os }}
29+
name: kvssink unit tests on ${{ matrix.run-config.os }}
30+
container: ${{ matrix.run-config.image }}
3031
steps:
3132
- name: Checkout repository
3233
uses: actions/checkout@v4
3334

3435
- name: Install dependencies
3536
run: |
36-
sudo apt-get update
37-
sudo apt-get install -y automake build-essential cmake git \
37+
apt-get update
38+
apt-get install -y automake m4 build-essential cmake git \
3839
gstreamer1.0-plugins-base-apps gstreamer1.0-plugins-bad \
3940
gstreamer1.0-plugins-good gstreamer1.0-plugins-ugly \
4041
gstreamer1.0-tools \
@@ -59,7 +60,7 @@ jobs:
5960
role-to-assume: ${{ secrets.AWS_ROLE_TO_ASSUME }}
6061
role-session-name: ${{ secrets.AWS_ROLE_SESSION_NAME }}
6162
aws-region: ${{ secrets.AWS_REGION }}
62-
role-duration-seconds: 10800
63+
role-duration-seconds: 900
6364

6465
- name: Run the unit tests
6566
working-directory: ./build
@@ -84,11 +85,16 @@ jobs:
8485

8586
mac-debug-dump-dir:
8687
runs-on: macos-latest
87-
env:
88-
AWS_KVS_LOG_LEVEL: 1
8988
permissions:
9089
id-token: write
9190
contents: read
91+
92+
env:
93+
AWS_KVS_LOG_LEVEL: 1
94+
STREAM_NAME: cpp-kvssink-debug-dump-dir-macos-latest
95+
GST_PLUGIN_PATH: ${{ github.workspace }}/build
96+
KVS_DEBUG_DUMP_DATA_FILE_DIR: ${{ github.workspace }}/build/debug_output
97+
9298
steps:
9399
- name: Clone repository
94100
uses: actions/checkout@v4
@@ -108,20 +114,17 @@ jobs:
108114
role-to-assume: ${{ secrets.AWS_ROLE_TO_ASSUME }}
109115
role-session-name: ${{ secrets.AWS_ROLE_SESSION_NAME }}
110116
aws-region: ${{ secrets.AWS_REGION }}
111-
role-duration-seconds: 10800
117+
role-duration-seconds: 900
112118

113119
- name: Run kvssink with dump dir
114-
env:
115-
GST_PLUGIN_PATH: ${{ github.workspace }}/build
116-
KVS_DEBUG_DUMP_DATA_FILE_DIR: ${{ github.workspace }}/build/debug_output
117120
working-directory: ./build
118121
run: |
119122
mkdir -p debug_output
120123
gst-launch-1.0 videotestsrc is-live=true num-buffers=450 \
121124
! video/x-raw,framerate=30/1,width=640,height=480 \
122125
! videoconvert ! x264enc tune=zerolatency key-int-max=45 \
123126
! h264parse \
124-
! kvssink stream-name="demo-stream"
127+
! kvssink stream-name="$STREAM_NAME"
125128
126129
- name: Verify MKV dump
127130
working-directory: ./build/debug_output
@@ -138,31 +141,37 @@ jobs:
138141
done
139142
140143
linux-debug-dump-dir:
141-
runs-on: ubuntu-latest
142-
env:
143-
AWS_KVS_LOG_LEVEL: 1
144144
permissions:
145145
id-token: write
146146
contents: read
147147
strategy:
148148
matrix:
149-
include:
150-
- os: Ubuntu 22.04
149+
run-config:
150+
- os: Ubuntu-22.04
151151
image: public.ecr.aws/ubuntu/ubuntu:22.04_stable
152-
- os: Ubuntu 20.04
152+
- os: Ubuntu-20.04
153153
image: public.ecr.aws/ubuntu/ubuntu:20.04_stable
154154
fail-fast: false
155+
156+
runs-on: ubuntu-latest
157+
container: ${{ matrix.run-config.image }}
158+
env:
159+
DEBIAN_FRONTEND: noninteractive
160+
STREAM_NAME: cpp-kvssink-debug-dump-dir-${{ matrix.run-config.os }}
161+
GST_PLUGIN_PATH: ${{ github.workspace }}/build
162+
KVS_DEBUG_DUMP_DATA_FILE_DIR: ${{ github.workspace }}/build/debug_output
163+
155164
steps:
156165
- name: Checkout repository
157166
uses: actions/checkout@v4
158167

159168
- name: Install dependencies
160169
run: |
161-
sudo apt-get update
162-
sudo apt-get install -y automake build-essential cmake git \
163-
gstreamer1.0-plugins-base-apps gstreamer1.0-plugins-bad \
164-
gstreamer1.0-plugins-good gstreamer1.0-plugins-ugly \
165-
gstreamer1.0-tools \
170+
apt-get update
171+
apt-get install -y automake build-essential cmake git \
172+
gstreamer1.0-plugins-base gstreamer1.0-plugins-base-apps \
173+
gstreamer1.0-plugins-bad gstreamer1.0-plugins-good \
174+
gstreamer1.0-plugins-ugly gstreamer1.0-tools \
166175
libcurl4-openssl-dev libgstreamer1.0-dev \
167176
libgstreamer-plugins-base1.0-dev liblog4cplus-dev \
168177
libssl-dev pkg-config mkvtoolnix
@@ -184,20 +193,17 @@ jobs:
184193
role-to-assume: ${{ secrets.AWS_ROLE_TO_ASSUME }}
185194
role-session-name: ${{ secrets.AWS_ROLE_SESSION_NAME }}
186195
aws-region: ${{ secrets.AWS_REGION }}
187-
role-duration-seconds: 10800
196+
role-duration-seconds: 900
188197

189198
- name: Run kvssink with dump dir
190-
env:
191-
GST_PLUGIN_PATH: ${{ github.workspace }}/build
192-
KVS_DEBUG_DUMP_DATA_FILE_DIR: ${{ github.workspace }}/build/debug_output
193199
working-directory: ./build
194200
run: |
195201
mkdir -p debug_output
196202
gst-launch-1.0 videotestsrc is-live=true num-buffers=450 \
197203
! video/x-raw,framerate=30/1,width=640,height=480 \
198204
! videoconvert ! x264enc tune=zerolatency key-int-max=45 \
199205
! h264parse \
200-
! kvssink stream-name="demo-stream"
206+
! kvssink stream-name="$STREAM_NAME"
201207
202208
- name: Verify MKV dump
203209
working-directory: ./build/debug_output
@@ -212,11 +218,13 @@ jobs:
212218
echo "Verifying $file with mkvinfo (verbose and hexdump):"
213219
mkvinfo -v -X "$file"
214220
done
221+
shell: bash
215222

216223
windows-debug-dump-dir:
217224
runs-on: windows-2022
218225
env:
219226
AWS_KVS_LOG_LEVEL: 1
227+
STREAM_NAME: cpp-kvssink-debug-dump-dir-windows-2022
220228
permissions:
221229
id-token: write
222230
contents: read
@@ -245,7 +253,7 @@ jobs:
245253
role-to-assume: ${{ secrets.AWS_ROLE_TO_ASSUME }}
246254
role-session-name: ${{ secrets.AWS_ROLE_SESSION_NAME }}
247255
aws-region: ${{ secrets.AWS_REGION }}
248-
role-duration-seconds: 10800
256+
role-duration-seconds: 900
249257
- name: Run kvssink with dump dir
250258
env:
251259
GST_PLUGIN_PATH: D:\producer\build\
@@ -258,7 +266,7 @@ jobs:
258266
New-Item -ItemType Directory -Path "D:\producer\build\debug_output" -Force
259267
260268
# Stream for 15 seconds (450 frames @ 30 fps)
261-
gst-launch-1.0.exe videotestsrc is-live=true num-buffers=450 ! video/x-raw,framerate=30/1,width=640,height=480 ! videoconvert ! x264enc tune=zerolatency key-int-max=45 ! h264parse ! kvssink stream-name="demo-stream"
269+
gst-launch-1.0.exe videotestsrc is-live=true num-buffers=450 ! video/x-raw,framerate=30/1,width=640,height=480 ! videoconvert ! x264enc tune=zerolatency key-int-max=45 ! h264parse ! kvssink stream-name="$STREAM_NAME"
262270
- name: Verify MKV dump
263271
working-directory: D:\producer\build
264272
run: |
@@ -301,6 +309,7 @@ jobs:
301309
build-essential
302310
cmake
303311
git
312+
gstreamer1.0-plugins-base
304313
gstreamer1.0-plugins-base-apps
305314
gstreamer1.0-plugins-bad
306315
gstreamer1.0-plugins-good
@@ -355,7 +364,7 @@ jobs:
355364
! video/x-raw,framerate=30/1,width=640,height=480 \
356365
! videoconvert ! x264enc tune=zerolatency key-int-max=45 \
357366
! h264parse \
358-
! kvssink stream-name='demo-stream-cpp-kvssink-ci-wsl-videotestsrc-${{ matrix.image }}' \
367+
! kvssink stream-name='cpp-kvssink-ci-wsl-videotestsrc-${{ matrix.image }}' \
359368
aws-region=${{ env.AWS_DEFAULT_REGION }} \
360369
access-key=${{ env.AWS_ACCESS_KEY_ID }} \
361370
secret-key=${{ env.AWS_SECRET_ACCESS_KEY }} \
@@ -380,3 +389,112 @@ jobs:
380389
echo "Verifying $file with mkvinfo:"
381390
mkvinfo -v -X "$file"
382391
done
392+
393+
valgrind-check:
394+
strategy:
395+
matrix:
396+
run-config:
397+
- os: Ubuntu-22.04
398+
image: public.ecr.aws/ubuntu/ubuntu:22.04_stable
399+
- os: Ubuntu-20.04
400+
image: public.ecr.aws/ubuntu/ubuntu:20.04_stable
401+
402+
deps:
403+
- name: Deps-ON
404+
cmake_flags: -DBUILD_DEPENDENCIES=ON
405+
- name: Deps-OFF
406+
cmake_flags: -DBUILD_DEPENDENCIES=OFF
407+
408+
fail-fast: false
409+
410+
runs-on: ubuntu-latest
411+
container: ${{ matrix.run-config.image }}
412+
413+
timeout-minutes: 30
414+
permissions:
415+
id-token: write
416+
contents: read
417+
418+
env:
419+
DEBIAN_FRONTEND: noninteractive
420+
STREAM_NAME: valgrind-test-stream-cpp-kvssink-${{ matrix.run-config.os }}-${{ matrix.deps.name }}
421+
GST_PLUGIN_PATH: ${{ github.workspace }}/build
422+
423+
name: Valgrind ${{ matrix.run-config.os }} (${{ matrix.deps.name }})
424+
steps:
425+
- name: Checkout repository
426+
uses: actions/checkout@v4
427+
428+
- name: Install dependencies
429+
run: |
430+
apt-get update
431+
apt-get install -y build-essential cmake pkg-config git \
432+
gstreamer1.0-plugins-base gstreamer1.0-plugins-base-apps \
433+
gstreamer1.0-plugins-bad gstreamer1.0-plugins-good \
434+
gstreamer1.0-plugins-ugly gstreamer1.0-tools \
435+
libgstreamer1.0-dev libgstreamer-plugins-base1.0-dev \
436+
valgrind
437+
438+
- name: Install additional build dependencies
439+
if: ${{ matrix.deps.name == 'Deps-ON' }}
440+
run: |
441+
apt-get install -y m4
442+
443+
- name: Install prebuilt-dependencies
444+
if: ${{ matrix.deps.name == 'Deps-OFF' }}
445+
run: |
446+
apt-get install -y libcurl4-openssl-dev liblog4cplus-dev libssl-dev
447+
448+
- name: Setup build directory
449+
run: |
450+
mkdir -p build
451+
cd build
452+
453+
- name: Build kvssink
454+
working-directory: ./build
455+
run: |
456+
cmake .. -DBUILD_GSTREAMER_PLUGIN=ON -DCMAKE_BUILD_TYPE=RelWithDebInfo -DALIGNED_MEMORY_MODEL=ON ${{ matrix.deps.cmake_flags }}
457+
make -j$(nproc)
458+
459+
- name: Configure AWS Credentials
460+
uses: aws-actions/configure-aws-credentials@v4
461+
with:
462+
role-to-assume: ${{ secrets.AWS_ROLE_TO_ASSUME }}
463+
role-session-name: ${{ secrets.AWS_ROLE_SESSION_NAME }}
464+
aws-region: ${{ secrets.AWS_REGION }}
465+
role-duration-seconds: 900
466+
467+
- name: Verify kvssink works
468+
working-directory: ./build
469+
run: |
470+
# Run it once just to confirm
471+
gst-launch-1.0 videotestsrc num-buffers=1 ! x264enc ! h264parse ! kvssink stream-name="$STREAM_NAME"
472+
473+
- name: Run kvssink with valgrind
474+
working-directory: ./build
475+
run: |
476+
mkdir -p valgrind_logs
477+
LOG_FILE="valgrind_logs/${{ matrix.sample-executable.name }}.log"
478+
479+
echo "::group::Application logs for kvssink"
480+
valgrind --leak-check=full \
481+
--show-leak-kinds=definite \
482+
--track-origins=yes \
483+
--log-file="$LOG_FILE" \
484+
--suppressions="../.github/valgrind/glib.supp" \
485+
gst-launch-1.0 videotestsrc num-buffers=1 \
486+
! x264enc \
487+
! h264parse \
488+
! kvssink stream-name="$STREAM_NAME" log-config="../.github/configs/no_logs_configuration"
489+
echo "::endgroup::"
490+
491+
echo "========== Valgrind Output =========="
492+
cat "$LOG_FILE"
493+
echo "====================================="
494+
495+
if grep -qE "definitely lost: [^0]" "$LOG_FILE" || grep -qE "indirectly lost: [^0]" "$LOG_FILE"; then
496+
echo "❌ Valgrind found something definitely or indirectly lost"
497+
exit 1
498+
fi
499+
500+
shell: bash

‎src/gstreamer/gstkvssink.cpp‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -788,7 +788,6 @@ gst_kvs_sink_init(GstKvsSink *kvssink) {
788788
static void
789789
gst_kvs_sink_finalize(GObject *object) {
790790
GstKvsSink *kvssink = GST_KVS_SINK (object);
791-
auto data = kvssink->data;
792791

793792
gst_object_unref(kvssink->collect);
794793
g_free(kvssink->stream_name);
@@ -806,18 +805,17 @@ gst_kvs_sink_finalize(GObject *object) {
806805
g_free(kvssink->credential_file_path);
807806

808807
if (kvssink->iot_certificate) {
809-
gst_structure_free (kvssink->iot_certificate);
808+
gst_structure_free(kvssink->iot_certificate);
810809
}
811810
if (kvssink->stream_tags) {
812-
gst_structure_free (kvssink->stream_tags);
813-
}
814-
if (data->kinesis_video_producer) {
815-
data->kinesis_video_producer.reset();
811+
gst_structure_free(kvssink->stream_tags);
816812
}
817-
818-
// Reset the shared pointer to properly release the KvsSinkCustomData
813+
814+
// Reset the smart pointers to properly release the data
815+
kvssink->credentials_.reset();
819816
kvssink->data.reset();
820-
817+
818+
// Eventually calls g_free(kvssink), smart pointers need to be cleaned up before
821819
G_OBJECT_CLASS (parent_class)->finalize(object);
822820
}
823821

‎src/gstreamer/gstkvssink.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,8 @@ struct _GstKvsSink {
141141
guint num_video_streams;
142142

143143

144+
// Since this struct is freed (not deleted), these pointers must be
145+
// manually cleaned up before freeing (finalize method)
144146
std::unique_ptr<Credentials> credentials_;
145147
std::shared_ptr<KvsSinkCustomData> data;
146148
};

0 commit comments

Comments
 (0)