From 9f1b945db9fd8af73871efa571c77baf8a608a48 Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Thu, 19 Oct 2023 22:12:18 -0700 Subject: [PATCH 1/2] fix: memory leak from string cstring disposal --- src/amplitude_experiment/local/evaluation/evaluation.py | 5 ++++- .../local/evaluation/libevaluation_interop.py | 6 +++++- tests/local/client_test.py | 7 ++++++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/amplitude_experiment/local/evaluation/evaluation.py b/src/amplitude_experiment/local/evaluation/evaluation.py index e4f426f..3454eb8 100644 --- a/src/amplitude_experiment/local/evaluation/evaluation.py +++ b/src/amplitude_experiment/local/evaluation/evaluation.py @@ -1,4 +1,5 @@ from .libevaluation_interop import libevaluation_interop_symbols +from ctypes import cast, c_char_p def evaluate(rules: str, user: str) -> str: """ @@ -11,4 +12,6 @@ def evaluate(rules: str, user: str) -> str: Evaluation results with variants in JSON """ result = libevaluation_interop_symbols().contents.kotlin.root.evaluate(rules, user) - return str(result, 'utf-8') + py_result = cast(result, c_char_p).value + libevaluation_interop_symbols().contents.DisposeString(result) + return str(py_result, 'utf-8') diff --git a/src/amplitude_experiment/local/evaluation/libevaluation_interop.py b/src/amplitude_experiment/local/evaluation/libevaluation_interop.py index 869e24f..de2b0c2 100644 --- a/src/amplitude_experiment/local/evaluation/libevaluation_interop.py +++ b/src/amplitude_experiment/local/evaluation/libevaluation_interop.py @@ -1086,7 +1086,11 @@ class struct_anon_14(Structure): 'evaluate', ] struct_anon_14._fields_ = [ - ('evaluate', CFUNCTYPE(UNCHECKED(c_char_p), String, String)), + # NOTE(bgiori): Changed this line from `UNCHECKED(c_char_p)` to `UNCHECKED(c_void_p)` + # to help fix a memory leak. Strings returned from kotlin/native must + # be freed using the DisposeString function, but c_char_p converts the + # c value making it incompatible with the dispose function. + ('evaluate', CFUNCTYPE(UNCHECKED(c_void_p), String, String)), ] # src/amplitude_experiment/local/evaluation/lib/macosX64/libevaluation_interop_api.h: 100 diff --git a/tests/local/client_test.py b/tests/local/client_test.py index 448afda..ae078d5 100644 --- a/tests/local/client_test.py +++ b/tests/local/client_test.py @@ -1,5 +1,5 @@ import unittest -from src.amplitude_experiment import LocalEvaluationClient, LocalEvaluationConfig, User, Variant +from src.amplitude_experiment import Experiment, LocalEvaluationClient, LocalEvaluationConfig, User, Variant, AssignmentConfig API_KEY = 'server-qz35UwzJ5akieoAdIgzM4m9MIiOLXLoz' test_user = User(user_id='test_user') @@ -56,6 +56,11 @@ def test_evaluate_with_dependencies_variant_holdout(self): expected_variant = None self.assertEqual(expected_variant, variants.get('sdk-local-evaluation-ci-test-holdout')) + def test(self): + variants = self._local_evaluation_client.evaluate(test_user) + v = variants.get('character-test') + print(v) + if __name__ == '__main__': unittest.main() From 9df569e35df6c7fc5618f4d09e6ffb19bf9edcbb Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Thu, 19 Oct 2023 22:15:10 -0700 Subject: [PATCH 2/2] undo test case --- tests/local/client_test.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/tests/local/client_test.py b/tests/local/client_test.py index ae078d5..448afda 100644 --- a/tests/local/client_test.py +++ b/tests/local/client_test.py @@ -1,5 +1,5 @@ import unittest -from src.amplitude_experiment import Experiment, LocalEvaluationClient, LocalEvaluationConfig, User, Variant, AssignmentConfig +from src.amplitude_experiment import LocalEvaluationClient, LocalEvaluationConfig, User, Variant API_KEY = 'server-qz35UwzJ5akieoAdIgzM4m9MIiOLXLoz' test_user = User(user_id='test_user') @@ -56,11 +56,6 @@ def test_evaluate_with_dependencies_variant_holdout(self): expected_variant = None self.assertEqual(expected_variant, variants.get('sdk-local-evaluation-ci-test-holdout')) - def test(self): - variants = self._local_evaluation_client.evaluate(test_user) - v = variants.get('character-test') - print(v) - if __name__ == '__main__': unittest.main()