From ff29b51fd947c203243988b5b374e36319e4799b Mon Sep 17 00:00:00 2001 From: xuzhenbao Date: Tue, 15 Feb 2022 20:33:11 +0800 Subject: [PATCH 01/11] fixed json rpc memory leak --- libs/dfi/gtest/src/json_rpc_tests.cpp | 27 +++++++++++ libs/dfi/src/json_rpc.c | 70 +++++++++++++-------------- libs/utils/include/celix_errno.h | 44 +++++++++++++++++ 3 files changed, 105 insertions(+), 36 deletions(-) diff --git a/libs/dfi/gtest/src/json_rpc_tests.cpp b/libs/dfi/gtest/src/json_rpc_tests.cpp index 8f8d744d8..f170911b6 100644 --- a/libs/dfi/gtest/src/json_rpc_tests.cpp +++ b/libs/dfi/gtest/src/json_rpc_tests.cpp @@ -98,6 +98,10 @@ static void stdLog(void*, int level, const char *file, int line, const char *msg return 0; } + int addFailed(void*, double , double , double *) { + return 0x02000001;// return customer error + } + int getName_example4(void*, char** result) { *result = strdup("allocatedInFunction"); return 0; @@ -198,6 +202,25 @@ static void stdLog(void*, int level, const char *file, int line, const char *msg dynInterface_destroy(intf); } + void callFailedTestPreAllocated(void) { + dyn_interface_type *intf = nullptr; + FILE *desc = fopen("descriptors/example1.descriptor", "r"); + ASSERT_TRUE(desc != nullptr); + int rc = dynInterface_parse(desc, &intf); + ASSERT_EQ(0, rc); + fclose(desc); + + char *result = nullptr; + tst_serv serv {nullptr, addFailed, nullptr, nullptr, nullptr}; + + rc = jsonRpc_call(intf, &serv, R"({"m":"add(DD)D", "a": [1.0,2.0]})", &result); + ASSERT_EQ(0, rc); + ASSERT_TRUE(strstr(result, "e") != nullptr); + + free(result); + dynInterface_destroy(intf); + } + void callTestOutput(void) { dyn_interface_type *intf = nullptr; FILE *desc = fopen("descriptors/example1.descriptor", "r"); @@ -404,6 +427,10 @@ TEST_F(JsonRpcTests, callPre) { callTestPreAllocated(); } +TEST_F(JsonRpcTests, callFailedPre) { + callFailedTestPreAllocated(); +} + TEST_F(JsonRpcTests, callOut) { callTestOutput(); } diff --git a/libs/dfi/src/json_rpc.c b/libs/dfi/src/json_rpc.c index edd9aa7a7..d2e39486b 100644 --- a/libs/dfi/src/json_rpc.c +++ b/libs/dfi/src/json_rpc.c @@ -177,52 +177,50 @@ int jsonRpc_call(dyn_interface_type *intf, void *service, const char *request, c } //serialize and free output - if (funcCallStatus == 0 && status == OK) { - for (i = 0; i < nrOfArgs; i += 1) { - dyn_type *argType = dynFunction_argumentTypeForIndex(func, i); - enum dyn_function_argument_meta meta = dynFunction_argumentMetaForIndex(func, i); - if (meta == DYN_FUNCTION_ARGUMENT_META__PRE_ALLOCATED_OUTPUT) { + for (i = 0; i < nrOfArgs; i += 1) { + dyn_type *argType = dynFunction_argumentTypeForIndex(func, i); + enum dyn_function_argument_meta meta = dynFunction_argumentMetaForIndex(func, i); + if (meta == DYN_FUNCTION_ARGUMENT_META__PRE_ALLOCATED_OUTPUT) { + if (funcCallStatus == 0 && status == OK) { + status = jsonSerializer_serializeJson(argType, args[i], &jsonResult); + } + dyn_type *subType = NULL; + dynType_typedPointer_getTypedType(argType, &subType); + void **ptrToInst = (void**)args[i]; + dynType_free(subType, *ptrToInst); + free(ptrToInst); + } else if (meta == DYN_FUNCTION_ARGUMENT_META__OUTPUT) { + if (funcCallStatus == 0 && ptr != NULL) { + dyn_type *typedType = NULL; if (status == OK) { - status = jsonSerializer_serializeJson(argType, args[i], &jsonResult); + status = dynType_typedPointer_getTypedType(argType, &typedType); } - dyn_type *subType = NULL; - dynType_typedPointer_getTypedType(argType, &subType); - void **ptrToInst = (void**)args[i]; - dynType_free(subType, *ptrToInst); - free(ptrToInst); - } else if (meta == DYN_FUNCTION_ARGUMENT_META__OUTPUT) { - if (ptr != NULL) { - dyn_type *typedType = NULL; + if (status == OK && dynType_descriptorType(typedType) == 't') { + status = jsonSerializer_serializeJson(typedType, (void*) &ptr, &jsonResult); + free(ptr); + } else { + dyn_type *typedTypedType = NULL; if (status == OK) { - status = dynType_typedPointer_getTypedType(argType, &typedType); + status = dynType_typedPointer_getTypedType(typedType, &typedTypedType); } - if (dynType_descriptorType(typedType) == 't') { - status = jsonSerializer_serializeJson(typedType, (void*) &ptr, &jsonResult); - free(ptr); - } else { - dyn_type *typedTypedType = NULL; - if (status == OK) { - status = dynType_typedPointer_getTypedType(typedType, &typedTypedType); - } - - if(status == OK){ - status = jsonSerializer_serializeJson(typedTypedType, ptr, &jsonResult); - } - - if (status == OK) { - dynType_free(typedTypedType, ptr); - } + + if(status == OK){ + status = jsonSerializer_serializeJson(typedTypedType, ptr, &jsonResult); } - } else { - LOG_DEBUG("Output ptr is null"); + if (status == OK) { + dynType_free(typedTypedType, ptr); + } } - } - if (status != OK) { - break; + } else { + LOG_DEBUG("Output ptr is null"); } } + + if (status != OK) { + break; + } } char *response = NULL; diff --git a/libs/utils/include/celix_errno.h b/libs/utils/include/celix_errno.h index 731af8288..4808e780e 100644 --- a/libs/utils/include/celix_errno.h +++ b/libs/utils/include/celix_errno.h @@ -102,6 +102,50 @@ const char* celix_strerror(celix_status_t status); #define CELIX_ENOMEM ENOMEM +/*! + * @brief Error code has 32bits, its internal structure as following + * + *|31-30bit|29bit|28-27bit|26-16bit|15-0bit| + *|--------|-----|--------|--------|-------| + *|R |C |R |Facility|Code | + * + * C (1bit): Customer. If set, indicates that the error code is customer-defined. If clear, indicates that the error code is celix-defines + * R : Reserved. It should be set to 0 + * Facility (11 bits): An indicator of the source of the error + * + */ + +/*! + * @brief Customer error code mask + * + */ +#define CELIX_CUSTOMER_ERR_MASK 0x02000000 + +/*! + * @brief The facility of system error code, + * @note Error code 0 indicates success,it is not system error code. + */ +#define CELIX_FACILITY_SYSTEM 0 + +/*! + * @brief The facility of celix default error code + * + */ +#define CELIX_FACILITY_NULL 1 + +/*! + * @brief The facility of the rpc subsystem error code + * + */ +#define CELIX_FACILITY_RPC 2 + +/*! + * @brief The facility of the http suppoter error code + * + */ +#define CELIX_FACILITY_HTTP 3 + + /** * \} */ From 004b9dad5cf233bba4ce3c105163b2079c8873d1 Mon Sep 17 00:00:00 2001 From: xuzhenbao Date: Wed, 16 Feb 2022 14:34:20 +0800 Subject: [PATCH 02/11] fixed json rpc memory leak --- libs/dfi/gtest/src/json_rpc_tests.cpp | 27 +++++++++++ libs/dfi/src/json_rpc.c | 70 +++++++++++++-------------- libs/utils/include/celix_errno.h | 44 +++++++++++++++++ 3 files changed, 105 insertions(+), 36 deletions(-) diff --git a/libs/dfi/gtest/src/json_rpc_tests.cpp b/libs/dfi/gtest/src/json_rpc_tests.cpp index 8f8d744d8..f170911b6 100644 --- a/libs/dfi/gtest/src/json_rpc_tests.cpp +++ b/libs/dfi/gtest/src/json_rpc_tests.cpp @@ -98,6 +98,10 @@ static void stdLog(void*, int level, const char *file, int line, const char *msg return 0; } + int addFailed(void*, double , double , double *) { + return 0x02000001;// return customer error + } + int getName_example4(void*, char** result) { *result = strdup("allocatedInFunction"); return 0; @@ -198,6 +202,25 @@ static void stdLog(void*, int level, const char *file, int line, const char *msg dynInterface_destroy(intf); } + void callFailedTestPreAllocated(void) { + dyn_interface_type *intf = nullptr; + FILE *desc = fopen("descriptors/example1.descriptor", "r"); + ASSERT_TRUE(desc != nullptr); + int rc = dynInterface_parse(desc, &intf); + ASSERT_EQ(0, rc); + fclose(desc); + + char *result = nullptr; + tst_serv serv {nullptr, addFailed, nullptr, nullptr, nullptr}; + + rc = jsonRpc_call(intf, &serv, R"({"m":"add(DD)D", "a": [1.0,2.0]})", &result); + ASSERT_EQ(0, rc); + ASSERT_TRUE(strstr(result, "e") != nullptr); + + free(result); + dynInterface_destroy(intf); + } + void callTestOutput(void) { dyn_interface_type *intf = nullptr; FILE *desc = fopen("descriptors/example1.descriptor", "r"); @@ -404,6 +427,10 @@ TEST_F(JsonRpcTests, callPre) { callTestPreAllocated(); } +TEST_F(JsonRpcTests, callFailedPre) { + callFailedTestPreAllocated(); +} + TEST_F(JsonRpcTests, callOut) { callTestOutput(); } diff --git a/libs/dfi/src/json_rpc.c b/libs/dfi/src/json_rpc.c index edd9aa7a7..d2e39486b 100644 --- a/libs/dfi/src/json_rpc.c +++ b/libs/dfi/src/json_rpc.c @@ -177,52 +177,50 @@ int jsonRpc_call(dyn_interface_type *intf, void *service, const char *request, c } //serialize and free output - if (funcCallStatus == 0 && status == OK) { - for (i = 0; i < nrOfArgs; i += 1) { - dyn_type *argType = dynFunction_argumentTypeForIndex(func, i); - enum dyn_function_argument_meta meta = dynFunction_argumentMetaForIndex(func, i); - if (meta == DYN_FUNCTION_ARGUMENT_META__PRE_ALLOCATED_OUTPUT) { + for (i = 0; i < nrOfArgs; i += 1) { + dyn_type *argType = dynFunction_argumentTypeForIndex(func, i); + enum dyn_function_argument_meta meta = dynFunction_argumentMetaForIndex(func, i); + if (meta == DYN_FUNCTION_ARGUMENT_META__PRE_ALLOCATED_OUTPUT) { + if (funcCallStatus == 0 && status == OK) { + status = jsonSerializer_serializeJson(argType, args[i], &jsonResult); + } + dyn_type *subType = NULL; + dynType_typedPointer_getTypedType(argType, &subType); + void **ptrToInst = (void**)args[i]; + dynType_free(subType, *ptrToInst); + free(ptrToInst); + } else if (meta == DYN_FUNCTION_ARGUMENT_META__OUTPUT) { + if (funcCallStatus == 0 && ptr != NULL) { + dyn_type *typedType = NULL; if (status == OK) { - status = jsonSerializer_serializeJson(argType, args[i], &jsonResult); + status = dynType_typedPointer_getTypedType(argType, &typedType); } - dyn_type *subType = NULL; - dynType_typedPointer_getTypedType(argType, &subType); - void **ptrToInst = (void**)args[i]; - dynType_free(subType, *ptrToInst); - free(ptrToInst); - } else if (meta == DYN_FUNCTION_ARGUMENT_META__OUTPUT) { - if (ptr != NULL) { - dyn_type *typedType = NULL; + if (status == OK && dynType_descriptorType(typedType) == 't') { + status = jsonSerializer_serializeJson(typedType, (void*) &ptr, &jsonResult); + free(ptr); + } else { + dyn_type *typedTypedType = NULL; if (status == OK) { - status = dynType_typedPointer_getTypedType(argType, &typedType); + status = dynType_typedPointer_getTypedType(typedType, &typedTypedType); } - if (dynType_descriptorType(typedType) == 't') { - status = jsonSerializer_serializeJson(typedType, (void*) &ptr, &jsonResult); - free(ptr); - } else { - dyn_type *typedTypedType = NULL; - if (status == OK) { - status = dynType_typedPointer_getTypedType(typedType, &typedTypedType); - } - - if(status == OK){ - status = jsonSerializer_serializeJson(typedTypedType, ptr, &jsonResult); - } - - if (status == OK) { - dynType_free(typedTypedType, ptr); - } + + if(status == OK){ + status = jsonSerializer_serializeJson(typedTypedType, ptr, &jsonResult); } - } else { - LOG_DEBUG("Output ptr is null"); + if (status == OK) { + dynType_free(typedTypedType, ptr); + } } - } - if (status != OK) { - break; + } else { + LOG_DEBUG("Output ptr is null"); } } + + if (status != OK) { + break; + } } char *response = NULL; diff --git a/libs/utils/include/celix_errno.h b/libs/utils/include/celix_errno.h index 731af8288..4808e780e 100644 --- a/libs/utils/include/celix_errno.h +++ b/libs/utils/include/celix_errno.h @@ -102,6 +102,50 @@ const char* celix_strerror(celix_status_t status); #define CELIX_ENOMEM ENOMEM +/*! + * @brief Error code has 32bits, its internal structure as following + * + *|31-30bit|29bit|28-27bit|26-16bit|15-0bit| + *|--------|-----|--------|--------|-------| + *|R |C |R |Facility|Code | + * + * C (1bit): Customer. If set, indicates that the error code is customer-defined. If clear, indicates that the error code is celix-defines + * R : Reserved. It should be set to 0 + * Facility (11 bits): An indicator of the source of the error + * + */ + +/*! + * @brief Customer error code mask + * + */ +#define CELIX_CUSTOMER_ERR_MASK 0x02000000 + +/*! + * @brief The facility of system error code, + * @note Error code 0 indicates success,it is not system error code. + */ +#define CELIX_FACILITY_SYSTEM 0 + +/*! + * @brief The facility of celix default error code + * + */ +#define CELIX_FACILITY_NULL 1 + +/*! + * @brief The facility of the rpc subsystem error code + * + */ +#define CELIX_FACILITY_RPC 2 + +/*! + * @brief The facility of the http suppoter error code + * + */ +#define CELIX_FACILITY_HTTP 3 + + /** * \} */ From fd46f4faac0181bc7ed3b5b106a5daa988a55a83 Mon Sep 17 00:00:00 2001 From: xuzhenbao Date: Wed, 16 Feb 2022 22:59:32 +0800 Subject: [PATCH 03/11] Get the invocation error of remote service function in client --- .../gtest/src/tst_activator.c | 2 +- .../src/import_registration_dfi.c | 19 +++-- .../src/import_registration_dfi.h | 2 +- .../src/remote_service_admin_dfi.c | 2 +- libs/dfi/gtest/src/json_rpc_avpr_tests.cpp | 8 +- libs/dfi/gtest/src/json_rpc_tests.cpp | 60 ++++++++++++- libs/dfi/include/json_rpc.h | 2 +- libs/dfi/src/json_rpc.c | 10 ++- libs/utils/include/celix_errno.h | 84 +++++++++---------- 9 files changed, 130 insertions(+), 59 deletions(-) diff --git a/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_activator.c b/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_activator.c index f8a9e2142..031731f6d 100644 --- a/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_activator.c +++ b/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_activator.c @@ -369,4 +369,4 @@ static celix_status_t bndStop(struct activator *act, celix_bundle_context_t* ctx return CELIX_SUCCESS; } -CELIX_GEN_BUNDLE_ACTIVATOR(struct activator, bndStart, bndStop); \ No newline at end of file +CELIX_GEN_BUNDLE_ACTIVATOR(struct activator, bndStart, bndStop); diff --git a/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c b/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c index 70221c24b..d3b6de1af 100644 --- a/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c +++ b/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c @@ -315,6 +315,8 @@ static void importRegistration_proxyFunc(void *userData, void *args[], void *ret struct method_entry *entry = userData; import_registration_t *import = *((void **)args[0]); + *(int *) returnVal = CELIX_SUCCESS; + if (import == NULL || import->send == NULL) { status = CELIX_ILLEGAL_ARGUMENT; } @@ -334,15 +336,21 @@ static void importRegistration_proxyFunc(void *userData, void *args[], void *ret celix_properties_t *metadata = NULL; bool cont = remoteInterceptorHandler_invokePreProxyCall(import->interceptorsHandler, import->endpoint->properties, entry->name, &metadata); if (cont) { - import->send(import->sendHandle, import->endpoint, invokeRequest, metadata, &reply, &rc); + status = import->send(import->sendHandle, import->endpoint, invokeRequest, metadata, &reply, &rc); //printf("request sended. got reply '%s' with status %i\n", reply, rc); - if (rc == 0 && dynFunction_hasReturn(entry->dynFunc)) { + if (status == CELIX_SUCCESS && rc == CELIX_SUCCESS && dynFunction_hasReturn(entry->dynFunc)) { //fjprintf("Handling reply '%s'\n", reply); - status = jsonRpc_handleReply(entry->dynFunc, reply, args); + int rsErrno = CELIX_SUCCESS; + status = jsonRpc_handleReply(entry->dynFunc, reply, args, &rsErrno); + if (rsErrno != CELIX_SUCCESS) { + //return the invocation error of remote service function + *(int *) returnVal = rsErrno; + } + } + else if (rc != CELIX_SUCCESS) { + *(int *) returnVal = rc; } - - *(int *) returnVal = rc; remoteInterceptorHandler_invokePostProxyCall(import->interceptorsHandler, import->endpoint->properties, entry->name, metadata); } @@ -362,6 +370,7 @@ static void importRegistration_proxyFunc(void *userData, void *args[], void *ret if (status != CELIX_SUCCESS) { //TODO log error + *(int *) returnVal = status; } } diff --git a/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.h b/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.h index 75d40bb8f..13353204f 100644 --- a/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.h +++ b/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.h @@ -25,7 +25,7 @@ #include -typedef void (*send_func_type)(void *handle, endpoint_description_t *endpointDescription, char *request, celix_properties_t *metadata, char **reply, int* replyStatus); +typedef celix_status_t (*send_func_type)(void *handle, endpoint_description_t *endpointDescription, char *request, celix_properties_t *metadata, char **reply, int* replyStatus); celix_status_t importRegistration_create( celix_bundle_context_t *context, diff --git a/bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c b/bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c index ced1a0802..5c6e52f99 100644 --- a/bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c +++ b/bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c @@ -957,7 +957,7 @@ static celix_status_t remoteServiceAdmin_send(void *handle, endpoint_description fputc('\0', get.stream); fclose(get.stream); *reply = get.buf; - *replyStatus = res; + *replyStatus = (res == CURLE_OK) ? CELIX_SUCCESS:CELIX_ERROR_MAKE(CELIX_FACILITY_HTTP,res); curl_easy_cleanup(curl); curl_slist_free_all(metadataHeader); diff --git a/libs/dfi/gtest/src/json_rpc_avpr_tests.cpp b/libs/dfi/gtest/src/json_rpc_avpr_tests.cpp index b302deec9..fb9c66b42 100644 --- a/libs/dfi/gtest/src/json_rpc_avpr_tests.cpp +++ b/libs/dfi/gtest/src/json_rpc_avpr_tests.cpp @@ -201,8 +201,10 @@ TEST_F(JsonAvprRpcTests, prep) { //double **out_ptr = &out; args[3] = &out; - rc = jsonRpc_handleReply(func, reply, args); + int rsErrno = 0; + rc = jsonRpc_handleReply(func, reply, args, &rsErrno); ASSERT_EQ(0, rc); + ASSERT_EQ(0, rsErrno); ASSERT_NEAR(2.2, calc_result, 0.001); free(result); @@ -255,8 +257,10 @@ TEST_F(JsonAvprRpcTests, handle_out) { void *out = &result; args[2] = &out; - int rc = jsonRpc_handleReply(func, reply, args); + int rsErrno = 0; + int rc = jsonRpc_handleReply(func, reply, args, &rsErrno); ASSERT_EQ(0, rc); + ASSERT_EQ(0, rsErrno); ASSERT_TRUE(result != nullptr); ASSERT_EQ(1.5, result->average); diff --git a/libs/dfi/gtest/src/json_rpc_tests.cpp b/libs/dfi/gtest/src/json_rpc_tests.cpp index f170911b6..ba83859cf 100644 --- a/libs/dfi/gtest/src/json_rpc_tests.cpp +++ b/libs/dfi/gtest/src/json_rpc_tests.cpp @@ -86,8 +86,10 @@ static void stdLog(void*, int level, const char *file, int line, const char *msg double *out = &result; void *args[4]; args[3] = &out; - rc = jsonRpc_handleReply(dynFunc, reply, args); + int rsErrno = 0; + rc = jsonRpc_handleReply(dynFunc, reply, args, &rsErrno); ASSERT_EQ(0, rc); + ASSERT_EQ(0, rsErrno); //ASSERT_EQ(2.2, result); dynFunction_destroy(dynFunc); @@ -271,8 +273,10 @@ static void stdLog(void*, int level, const char *file, int line, const char *msg void *out = &result; args[2] = &out; - rc = jsonRpc_handleReply(func, reply, args); + int rsErrno = 0; + rc = jsonRpc_handleReply(func, reply, args, &rsErrno); ASSERT_EQ(0, rc); + ASSERT_EQ(0, rsErrno); ASSERT_EQ(1.5, result->average); free(result->input.buf); @@ -280,6 +284,47 @@ static void stdLog(void*, int level, const char *file, int line, const char *msg dynInterface_destroy(intf); } + void handleTestReplyError(void) { + dyn_interface_type *intf = nullptr; + FILE *desc = fopen("descriptors/example1.descriptor", "r"); + ASSERT_TRUE(desc != nullptr); + int rc = dynInterface_parse(desc, &intf); + ASSERT_EQ(0, rc); + fclose(desc); + + struct methods_head *head; + dynInterface_methods(intf, &head); + dyn_function_type *func = nullptr; + struct method_entry *entry = nullptr; + TAILQ_FOREACH(entry, head, entries) { + if (strcmp(entry->name, "add") == 0) { + func = entry->dynFunc; + break; + } + } + ASSERT_TRUE(func != nullptr); + + const char *reply = R"({"e":33554433})"; + + void *args[4]; + args[0] = nullptr; + args[1] = nullptr; + args[2] = nullptr; + args[3] = nullptr; + + double result = 0; + void *out = &result; + args[3] = &out; + + int rsErrno = 0; + rc = jsonRpc_handleReply(func, reply, args, &rsErrno); + ASSERT_EQ(0, rc); + ASSERT_EQ(33554433, rsErrno); + ASSERT_EQ(0, result); + + dynInterface_destroy(intf); + } + static void handleTestOutputSequence() { dyn_interface_type *intf = nullptr; FILE *desc = fopen("descriptors/example2.descriptor", "r"); @@ -313,8 +358,10 @@ static void stdLog(void*, int level, const char *file, int line, const char *msg void *out = &result; args[1] = &out; - rc = jsonRpc_handleReply(func, reply, args); + int rsErrno = 0; + rc = jsonRpc_handleReply(func, reply, args, &rsErrno); ASSERT_EQ(0, rc); + ASSERT_EQ(0, rsErrno); ASSERT_EQ(2, result->len); ASSERT_EQ(1.0, result->buf[0]->a); ASSERT_EQ(1.5, result->buf[0]->b); @@ -385,7 +432,9 @@ static void stdLog(void*, int level, const char *file, int line, const char *msg args[1] = &out; if (func != nullptr) { // Check needed just to satisfy Coverity - jsonRpc_handleReply(func, reply, args); + int rsErrno = 0; + jsonRpc_handleReply(func, reply, args, &rsErrno); + ASSERT_EQ(0, rsErrno); } ASSERT_STREQ("this is a test string", result); @@ -447,3 +496,6 @@ TEST_F(JsonRpcTests, handleOutChar) { handleTestOutChar(); } +TEST_F(JsonRpcTests, handleReplyError) { + handleTestReplyError(); +} diff --git a/libs/dfi/include/json_rpc.h b/libs/dfi/include/json_rpc.h index f2a729157..363d051e7 100644 --- a/libs/dfi/include/json_rpc.h +++ b/libs/dfi/include/json_rpc.h @@ -37,7 +37,7 @@ int jsonRpc_call(dyn_interface_type *intf, void *service, const char *request, c int jsonRpc_prepareInvokeRequest(dyn_function_type *func, const char *id, void *args[], char **out); -int jsonRpc_handleReply(dyn_function_type *func, const char *reply, void *args[]); +int jsonRpc_handleReply(dyn_function_type *func, const char *reply, void *args[], int *rsErrno); #ifdef __cplusplus } diff --git a/libs/dfi/src/json_rpc.c b/libs/dfi/src/json_rpc.c index d2e39486b..7203a43e5 100644 --- a/libs/dfi/src/json_rpc.c +++ b/libs/dfi/src/json_rpc.c @@ -304,7 +304,7 @@ int jsonRpc_prepareInvokeRequest(dyn_function_type *func, const char *id, void * return status; } -int jsonRpc_handleReply(dyn_function_type *func, const char *reply, void *args[]) { +int jsonRpc_handleReply(dyn_function_type *func, const char *reply, void *args[], int *rsErrno) { int status = OK; json_error_t error; @@ -315,11 +315,19 @@ int jsonRpc_handleReply(dyn_function_type *func, const char *reply, void *args[] } json_t *result = NULL; + json_t *rsError = NULL; bool replyHasResult = false; if (status == OK) { + *rsErrno = 0; result = json_object_get(replyJson, "r"); if (result != NULL) { replyHasResult = true; + } else { + rsError = json_object_get(replyJson, "e"); + if(rsError != NULL) { + //get the invocation error of remote service function + *rsErrno = json_integer_value(rsError); + } } } diff --git a/libs/utils/include/celix_errno.h b/libs/utils/include/celix_errno.h index 4808e780e..038acd2cc 100644 --- a/libs/utils/include/celix_errno.h +++ b/libs/utils/include/celix_errno.h @@ -21,6 +21,15 @@ * \file * \brief Error codes * \defgroup framework Celix Framework + * \note Error code has 32bits, its internal structure as following + * + *|31-30bit|29bit|28-27bit|26-16bit|15-0bit| + *|--------|-----|--------|--------|-------| + *|R |C |R |Facility|Code | + * + * C (1bit): Customer. If set, indicates that the error code is customer-defined. If clear, indicates that the error code is celix-defines + * R : Reserved. It should be set to 0 + * Facility (11 bits): An indicator of the source of the error */ #ifndef CELIX_ERRNO_H_ #define CELIX_ERRNO_H_ @@ -61,6 +70,38 @@ typedef int celix_status_t; */ const char* celix_strerror(celix_status_t status); +/*! + * \brief Customer error code mask + * + */ +#define CELIX_CUSTOMER_ERR_MASK 0x02000000 + +/*! + * \brief The facility of system error code, + * \note Error code 0 indicates success,it is not system error code. + */ +#define CELIX_FACILITY_SYSTEM 0 + +/*! + * \brief The facility of celix default error code + * + */ +#define CELIX_FACILITY_NULL 1 + +/*! + * \brief The facility of the http suppoter error code + * + */ +#define CELIX_FACILITY_HTTP 2 + +/*! + * \brief Make the error code accroding to the specification + * \param fac Facility + * \param code Code + */ +#define CELIX_ERROR_MAKE(fac,code) (((unsigned int)(fac)<<16) | ((code)&0xFFFF)) + + /*! * Error code indicating successful execution of the function. */ @@ -102,49 +143,6 @@ const char* celix_strerror(celix_status_t status); #define CELIX_ENOMEM ENOMEM -/*! - * @brief Error code has 32bits, its internal structure as following - * - *|31-30bit|29bit|28-27bit|26-16bit|15-0bit| - *|--------|-----|--------|--------|-------| - *|R |C |R |Facility|Code | - * - * C (1bit): Customer. If set, indicates that the error code is customer-defined. If clear, indicates that the error code is celix-defines - * R : Reserved. It should be set to 0 - * Facility (11 bits): An indicator of the source of the error - * - */ - -/*! - * @brief Customer error code mask - * - */ -#define CELIX_CUSTOMER_ERR_MASK 0x02000000 - -/*! - * @brief The facility of system error code, - * @note Error code 0 indicates success,it is not system error code. - */ -#define CELIX_FACILITY_SYSTEM 0 - -/*! - * @brief The facility of celix default error code - * - */ -#define CELIX_FACILITY_NULL 1 - -/*! - * @brief The facility of the rpc subsystem error code - * - */ -#define CELIX_FACILITY_RPC 2 - -/*! - * @brief The facility of the http suppoter error code - * - */ -#define CELIX_FACILITY_HTTP 3 - /** * \} From a456c47840f1cd70e7f43cc89ac7fe68417af6e7 Mon Sep 17 00:00:00 2001 From: xuzhenbao Date: Thu, 17 Feb 2022 12:02:56 +0800 Subject: [PATCH 04/11] Modify error code comments format --- libs/utils/include/celix_errno.h | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/libs/utils/include/celix_errno.h b/libs/utils/include/celix_errno.h index 038acd2cc..94b499620 100644 --- a/libs/utils/include/celix_errno.h +++ b/libs/utils/include/celix_errno.h @@ -21,15 +21,18 @@ * \file * \brief Error codes * \defgroup framework Celix Framework - * \note Error code has 32bits, its internal structure as following + * \details Error code has 32bits. If the value of error code is 0,it indicates success;if no-zero,it indicates error. + * its internal structure as following, * - *|31-30bit|29bit|28-27bit|26-16bit|15-0bit| - *|--------|-----|--------|--------|-------| - *|R |C |R |Facility|Code | + * |31-30bit|29bit|28-27bit|26-16bit|15-0bit| + * |-------:|:---:|:------:|:------:|:------| + * |R |C |R |Facility|Code | * - * C (1bit): Customer. If set, indicates that the error code is customer-defined. If clear, indicates that the error code is celix-defines - * R : Reserved. It should be set to 0 - * Facility (11 bits): An indicator of the source of the error + * - C (1bit): Customer. If set, indicates that the error code is customer-defined. + * If clear, indicates that the error code is celix-defines. + * - R : Reserved. It should be set to 0. + * - Facility (11 bits): An indicator of the source of the error. + * - Code (16bits): The remainder of error code. */ #ifndef CELIX_ERRNO_H_ #define CELIX_ERRNO_H_ @@ -71,31 +74,31 @@ typedef int celix_status_t; const char* celix_strerror(celix_status_t status); /*! - * \brief Customer error code mask + * Customer error code mask * */ #define CELIX_CUSTOMER_ERR_MASK 0x02000000 /*! - * \brief The facility of system error code, + * The facility of system error code, * \note Error code 0 indicates success,it is not system error code. */ #define CELIX_FACILITY_SYSTEM 0 /*! - * \brief The facility of celix default error code + * The facility of celix default error code * */ #define CELIX_FACILITY_NULL 1 /*! - * \brief The facility of the http suppoter error code + * The facility of the http suppoter error code * */ #define CELIX_FACILITY_HTTP 2 /*! - * \brief Make the error code accroding to the specification + * Make the error code accroding to the specification * \param fac Facility * \param code Code */ From 01290d49cdc7166a71c6a5536e7b0369414f09a5 Mon Sep 17 00:00:00 2001 From: xuzhenbao Date: Thu, 17 Feb 2022 12:25:04 +0800 Subject: [PATCH 05/11] record CELIX_START_USERERR deprecated --- libs/utils/include/celix_errno.h | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/utils/include/celix_errno.h b/libs/utils/include/celix_errno.h index 94b499620..3de18f248 100644 --- a/libs/utils/include/celix_errno.h +++ b/libs/utils/include/celix_errno.h @@ -122,6 +122,7 @@ const char* celix_strerror(celix_status_t status); /*! * The start error number user application can use. + * \deprecated It is recommended to use CELIX_CUSTOMER_ERR_MASK to define user error number */ #define CELIX_START_USERERR (CELIX_START_ERROR + CELIX_ERRSPACE_SIZE) From c0fb07455d4f64a2a503760e9991ca106993f55f Mon Sep 17 00:00:00 2001 From: xuzhenbao Date: Thu, 17 Feb 2022 15:59:07 +0800 Subject: [PATCH 06/11] service proxy handle interceptor exception --- .../remote_service_admin_dfi/src/import_registration_dfi.c | 5 +++-- libs/utils/include/celix_errno.h | 5 +++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c b/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c index d3b6de1af..8a6f51e76 100644 --- a/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c +++ b/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c @@ -347,12 +347,13 @@ static void importRegistration_proxyFunc(void *userData, void *args[], void *ret //return the invocation error of remote service function *(int *) returnVal = rsErrno; } - } - else if (rc != CELIX_SUCCESS) { + } else if (rc != CELIX_SUCCESS) { *(int *) returnVal = rc; } remoteInterceptorHandler_invokePostProxyCall(import->interceptorsHandler, import->endpoint->properties, entry->name, metadata); + } else { + *(int *) returnVal = CELIX_INTERCEPTOR_EXCEPTION; } if (import->logFile != NULL) { diff --git a/libs/utils/include/celix_errno.h b/libs/utils/include/celix_errno.h index 3de18f248..d13c47747 100644 --- a/libs/utils/include/celix_errno.h +++ b/libs/utils/include/celix_errno.h @@ -145,6 +145,11 @@ const char* celix_strerror(celix_status_t status); #define CELIX_FILE_IO_EXCEPTION (CELIX_START_ERROR + 8) #define CELIX_SERVICE_EXCEPTION (CELIX_START_ERROR + 9) +/*! + * Exception indicating a problem with a interceptor + */ +#define CELIX_INTERCEPTOR_EXCEPTION (CELIX_START_ERROR + 10) + #define CELIX_ENOMEM ENOMEM From 490881134c27886ca6f5dcd3defcd3394e748012 Mon Sep 17 00:00:00 2001 From: xuzhenbao Date: Mon, 21 Feb 2022 14:18:22 +0800 Subject: [PATCH 07/11] fix memory leak for interceptor callback return false --- .../gtest/CMakeLists.txt | 2 + .../gtest/exception_test_service.descriptor | 9 + .../gtest/src/rsa_client_server_tests.cc | 177 ++++++++++++++++++ .../src/export_registration_dfi.c | 6 +- .../src/export_registration_dfi.h | 2 +- .../src/import_registration_dfi.c | 5 + .../src/remote_service_admin_dfi.c | 9 +- .../src/remote_interceptors_handler.c | 3 +- libs/dfi/gtest/src/json_rpc_tests.cpp | 3 +- libs/utils/include/celix_errno.h | 8 +- 10 files changed, 214 insertions(+), 10 deletions(-) create mode 100644 bundles/remote_services/remote_service_admin_dfi/gtest/exception_test_service.descriptor diff --git a/bundles/remote_services/remote_service_admin_dfi/gtest/CMakeLists.txt b/bundles/remote_services/remote_service_admin_dfi/gtest/CMakeLists.txt index f3931c74b..aa33865f8 100644 --- a/bundles/remote_services/remote_service_admin_dfi/gtest/CMakeLists.txt +++ b/bundles/remote_services/remote_service_admin_dfi/gtest/CMakeLists.txt @@ -57,6 +57,8 @@ get_property(remote_example_bundle_file TARGET remote_example_service PROPERTY B configure_file(config.properties.in config.properties) configure_file(client.properties.in client.properties) configure_file(server.properties.in server.properties) +#add exception service interface descriptor +configure_file(exception_test_service.descriptor exception_test_service.descriptor) add_dependencies(test_rsa_dfi rsa_dfi_bundle #note depend on the target creating the bundle zip not the lib target diff --git a/bundles/remote_services/remote_service_admin_dfi/gtest/exception_test_service.descriptor b/bundles/remote_services/remote_service_admin_dfi/gtest/exception_test_service.descriptor new file mode 100644 index 000000000..957e05ffc --- /dev/null +++ b/bundles/remote_services/remote_service_admin_dfi/gtest/exception_test_service.descriptor @@ -0,0 +1,9 @@ +:header +type=interface +name=exception_test +version=1.0.0 +:annotations +classname=exception_test_service +:types +:methods +func1(V)V=func1(#am=handle;P)N diff --git a/bundles/remote_services/remote_service_admin_dfi/gtest/src/rsa_client_server_tests.cc b/bundles/remote_services/remote_service_admin_dfi/gtest/src/rsa_client_server_tests.cc index a47e3ed29..549ba1007 100644 --- a/bundles/remote_services/remote_service_admin_dfi/gtest/src/rsa_client_server_tests.cc +++ b/bundles/remote_services/remote_service_admin_dfi/gtest/src/rsa_client_server_tests.cc @@ -34,6 +34,13 @@ extern "C" { #include "celix_launcher.h" #include "framework.h" #include "remote_service_admin.h" +#include "remote_interceptor.h" + +#define RSA_DIF_EXCEPTION_TEST_SERVICE "exception_test_service" +typedef struct rsa_dfi_exception_test_service { + void *handle; + int (*func1)(void *handle); +}rsa_dfi_exception_test_service_t; static celix_framework_t *serverFramework = NULL; static celix_bundle_context_t *serverContext = NULL; @@ -41,6 +48,15 @@ extern "C" { static celix_framework_t *clientFramework = NULL; static celix_bundle_context_t *clientContext = NULL; + static rsa_dfi_exception_test_service_t *exceptionTestService = NULL; + static long exceptionTestSvcId = -1L; + static remote_interceptor_t *serverSvcInterceptor=NULL; + static remote_interceptor_t *clientSvcInterceptor=NULL; + static long serverSvcInterceptorSvcId = -1L; + static long clientSvcInterceptorSvcId = -1L; + static bool clientInterceptorPreProxyCallRetval=true; + static bool svcInterceptorPreExportCallRetval=true; + static void setupFm(bool useCurlShare) { //server celix_properties_t *serverProps = celix_properties_load("server.properties"); @@ -65,6 +81,101 @@ extern "C" { celix_frameworkFactory_destroyFramework(clientFramework); } + static int rsaDfi_excepTestFunc1(void *handle __attribute__((unused))) { + return CELIX_CUSTOMER_ERROR_MAKE(0,1); + } + + static void registerExceptionTestServer(void) { + celix_properties_t *properties = celix_properties_create(); + celix_properties_set(properties, OSGI_RSA_SERVICE_EXPORTED_INTERFACES, RSA_DIF_EXCEPTION_TEST_SERVICE); + celix_properties_set(properties, OSGI_RSA_SERVICE_EXPORTED_CONFIGS, "org.amdatu.remote.admin.http"); + exceptionTestService = (rsa_dfi_exception_test_service_t *)calloc(1,sizeof(*exceptionTestService)); + exceptionTestService->handle = NULL; + exceptionTestService->func1 = rsaDfi_excepTestFunc1; + exceptionTestSvcId = celix_bundleContext_registerService(serverContext, exceptionTestService, RSA_DIF_EXCEPTION_TEST_SERVICE, properties); + } + + static void unregisterExceptionTestServer(void) { + celix_bundleContext_unregisterService(serverContext, exceptionTestSvcId); + free(exceptionTestService); + } + + static bool serverServiceInterceptor_preProxyCall(void *, const celix_properties_t *, const char *, celix_properties_t *) { + return true; + } + + static void serverServiceInterceptor_postProxyCall(void *, const celix_properties_t *, const char *, celix_properties_t *) { + + } + + static bool serverServiceInterceptor_preExportCall(void *, const celix_properties_t *, const char *, celix_properties_t *) { + return svcInterceptorPreExportCallRetval; + } + + static void serverServiceInterceptor_postExportCall(void *, const celix_properties_t *, const char *, celix_properties_t *) { + + } + + static bool clientServiceInterceptor_preProxyCall(void *, const celix_properties_t *, const char *, celix_properties_t *) { + return clientInterceptorPreProxyCallRetval; + } + + static void clientServiceInterceptor_postProxyCall(void *, const celix_properties_t *, const char *, celix_properties_t *) { + + } + + static bool clientServiceInterceptor_preExportCall(void *, const celix_properties_t *, const char *, celix_properties_t *) { + return true; + } + + static void clientServiceInterceptor_postExportCall(void *, const celix_properties_t *, const char *, celix_properties_t *) { + + } + + static void registerInterceptorService(void) { + svcInterceptorPreExportCallRetval = true; + serverSvcInterceptor = (remote_interceptor_t *)calloc(1,sizeof(*serverSvcInterceptor)); + serverSvcInterceptor->handle = NULL; + serverSvcInterceptor->preProxyCall = serverServiceInterceptor_preProxyCall; + serverSvcInterceptor->postProxyCall = serverServiceInterceptor_postProxyCall; + serverSvcInterceptor->preExportCall = serverServiceInterceptor_preExportCall; + serverSvcInterceptor->postExportCall = serverServiceInterceptor_postExportCall; + celix_properties_t *svcInterceptorProps = celix_properties_create(); + celix_properties_setLong(svcInterceptorProps, OSGI_FRAMEWORK_SERVICE_RANKING, 10); + celix_service_registration_options_t svcInterceptorOpts{}; + memset(&svcInterceptorOpts,0,sizeof(svcInterceptorOpts)); + svcInterceptorOpts.svc = serverSvcInterceptor; + svcInterceptorOpts.serviceName = REMOTE_INTERCEPTOR_SERVICE_NAME; + svcInterceptorOpts.serviceVersion = REMOTE_INTERCEPTOR_SERVICE_VERSION; + svcInterceptorOpts.properties = svcInterceptorProps; + serverSvcInterceptorSvcId = celix_bundleContext_registerServiceWithOptions(serverContext, &svcInterceptorOpts); + + clientInterceptorPreProxyCallRetval = true; + clientSvcInterceptor = (remote_interceptor_t *)calloc(1,sizeof(*clientSvcInterceptor)); + clientSvcInterceptor->handle = NULL; + clientSvcInterceptor->preProxyCall = clientServiceInterceptor_preProxyCall; + clientSvcInterceptor->postProxyCall = clientServiceInterceptor_postProxyCall; + clientSvcInterceptor->preExportCall = clientServiceInterceptor_preExportCall; + clientSvcInterceptor->postExportCall = clientServiceInterceptor_postExportCall; + celix_properties_t *clientInterceptorProps = celix_properties_create(); + celix_properties_setLong(clientInterceptorProps, OSGI_FRAMEWORK_SERVICE_RANKING, 10); + celix_service_registration_options_t clientInterceptorOpts{}; + memset(&clientInterceptorOpts,0,sizeof(clientInterceptorOpts)); + clientInterceptorOpts.svc = clientSvcInterceptor; + clientInterceptorOpts.serviceName = REMOTE_INTERCEPTOR_SERVICE_NAME; + clientInterceptorOpts.serviceVersion = REMOTE_INTERCEPTOR_SERVICE_VERSION; + clientInterceptorOpts.properties = clientInterceptorProps; + clientSvcInterceptorSvcId = celix_bundleContext_registerServiceWithOptions(clientContext, &clientInterceptorOpts); + } + + static void unregisterInterceptorService(void) { + celix_bundleContext_unregisterService(clientContext, clientSvcInterceptorSvcId); + free(clientSvcInterceptor); + + celix_bundleContext_unregisterService(serverContext, serverSvcInterceptorSvcId); + free(serverSvcInterceptor); + } + static void testComplex(void *handle __attribute__((unused)), void *svc) { auto *tst = static_cast(svc); @@ -149,6 +260,37 @@ extern "C" { ASSERT_TRUE(ok); }; + static void testInterceptorPreExportCallReturnFalse(void *handle __attribute__((unused)), void *svc) { + svcInterceptorPreExportCallRetval = false; + auto *tst = static_cast(svc); + + bool ok = tst->testRemoteAction(tst->handle); + ASSERT_FALSE(ok); + } + + static void testInterceptorPreProxyCallReturnFalse(void *handle __attribute__((unused)), void *svc) { + clientInterceptorPreProxyCallRetval = false; + auto *tst = static_cast(svc); + + bool ok = tst->testRemoteAction(tst->handle); + ASSERT_FALSE(ok); + } + + static void testExceptionServiceCallback(void *handle __attribute__((unused)), void *svc) { + rsa_dfi_exception_test_service_t * service = (rsa_dfi_exception_test_service_t *)(svc); + int ret = service->func1(service->handle); + EXPECT_EQ(CELIX_CUSTOMER_ERROR_MAKE(0,1),ret); + } + + static void testExceptionService(void) { + celix_service_use_options_t opts{}; + opts.filter.serviceName = RSA_DIF_EXCEPTION_TEST_SERVICE; + opts.use = testExceptionServiceCallback; + opts.filter.ignoreServiceLanguage = true; + opts.waitTimeoutInSeconds = 2; + bool called = celix_bundleContext_useServiceWithOptions(clientContext, &opts); + ASSERT_TRUE(called); + } } template @@ -184,6 +326,29 @@ class RsaDfiClientServerWithCurlShareTests : public ::testing::Test { }; +class RsaDfiClientServerInterceptorTests : public ::testing::Test { +public: + RsaDfiClientServerInterceptorTests() { + setupFm(false); + registerInterceptorService(); + } + ~RsaDfiClientServerInterceptorTests() override { + unregisterInterceptorService(); + teardownFm(); + } +}; + +class RsaDfiClientServerExceptionTests : public ::testing::Test { +public: + RsaDfiClientServerExceptionTests() { + setupFm(false); + registerExceptionTestServer(); + } + ~RsaDfiClientServerExceptionTests() override { + unregisterExceptionTestServer(); + teardownFm(); + } +}; TEST_F(RsaDfiClientServerTests, TestRemoteCalculator) { test(testCalculator); @@ -228,3 +393,15 @@ TEST_F(RsaDfiClientServerTests, CreateDestroyComponentWithRemoteService) { TEST_F(RsaDfiClientServerTests, AddRemoteServiceInRemoteService) { test(testAddRemoteServiceInRemoteService); } + +TEST_F(RsaDfiClientServerInterceptorTests,TestInterceptorPreExportCallReturnFalse) { + test(testInterceptorPreExportCallReturnFalse); +} + +TEST_F(RsaDfiClientServerInterceptorTests,TestInterceptorPreProxyCallReturnFalse) { + test(testInterceptorPreProxyCallReturnFalse); +} + +TEST_F(RsaDfiClientServerExceptionTests,TestExceptionService) { + testExceptionService(); +} diff --git a/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.c b/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.c index 7af2cd953..5acd02f2b 100644 --- a/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.c +++ b/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.c @@ -154,7 +154,7 @@ void exportRegistration_waitTillNotUsed(export_registration_t *export) { celixThreadMutex_unlock(&export->mutex); } -celix_status_t exportRegistration_call(export_registration_t *export, char *data, int datalength, celix_properties_t *metadata, char **responseOut, int *responseLength) { +celix_status_t exportRegistration_call(export_registration_t *export, char *data, int datalength, celix_properties_t **metadata, char **responseOut, int *responseLength) { int status = CELIX_SUCCESS; char* response = NULL; @@ -164,7 +164,7 @@ celix_status_t exportRegistration_call(export_registration_t *export, char *data const char *sig; if (js_request) { if (json_unpack(js_request, "{s:s}", "m", &sig) == 0) { - bool cont = remoteInterceptorHandler_invokePreExportCall(export->interceptorsHandler, export->exportReference.endpoint->properties, sig, &metadata); + bool cont = remoteInterceptorHandler_invokePreExportCall(export->interceptorsHandler, export->exportReference.endpoint->properties, sig, metadata); if (cont) { celixThreadMutex_lock(&export->mutex); if (export->active && export->service != NULL) { @@ -178,7 +178,7 @@ celix_status_t exportRegistration_call(export_registration_t *export, char *data } celixThreadMutex_unlock(&export->mutex); - remoteInterceptorHandler_invokePostExportCall(export->interceptorsHandler, export->exportReference.endpoint->properties, sig, metadata); + remoteInterceptorHandler_invokePostExportCall(export->interceptorsHandler, export->exportReference.endpoint->properties, sig, *metadata); } *responseOut = response; diff --git a/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.h b/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.h index b07b8699d..37809df74 100644 --- a/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.h +++ b/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.h @@ -31,7 +31,7 @@ void exportRegistration_destroy(export_registration_t *registration); celix_status_t exportRegistration_start(export_registration_t *registration); void exportRegistration_setActive(export_registration_t *registration, bool active); -celix_status_t exportRegistration_call(export_registration_t *export, char *data, int datalength, celix_properties_t *metadata, char **response, int *responseLength); +celix_status_t exportRegistration_call(export_registration_t *export, char *data, int datalength, celix_properties_t **metadata, char **response, int *responseLength); void exportRegistration_increaseUsage(export_registration_t *export); void exportRegistration_decreaseUsage(export_registration_t *export); diff --git a/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c b/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c index 8a6f51e76..1285162a9 100644 --- a/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c +++ b/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c @@ -356,6 +356,11 @@ static void importRegistration_proxyFunc(void *userData, void *args[], void *ret *(int *) returnVal = CELIX_INTERCEPTOR_EXCEPTION; } + //free metadata + if(metadata != NULL) { + celix_properties_destroy(metadata); + } + if (import->logFile != NULL) { static int callCount = 0; const char *url = importRegistration_getUrl(import); diff --git a/bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c b/bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c index 5c6e52f99..97dfb001c 100644 --- a/bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c +++ b/bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c @@ -501,7 +501,7 @@ static int remoteServiceAdmin_callback(struct mg_connection *conn) { char *response = NULL; int responceLength = 0; - int rc = exportRegistration_call(export, data, -1, metadata, &response, &responceLength); + int rc = exportRegistration_call(export, data, -1, &metadata, &response, &responceLength); if (rc != CELIX_SUCCESS) { RSA_LOG_ERROR(rsa, "Error trying to invoke remove service, got error %i\n", rc); } @@ -534,11 +534,14 @@ static int remoteServiceAdmin_callback(struct mg_connection *conn) { free(data); exportRegistration_decreaseUsage(export); - - //TODO free metadata? } } + //free metadata + if(metadata != NULL) { + celix_properties_destroy(metadata); + } + return result; } diff --git a/bundles/remote_services/rsa_common/src/remote_interceptors_handler.c b/bundles/remote_services/rsa_common/src/remote_interceptors_handler.c index c1f830f50..c8bb1e75a 100644 --- a/bundles/remote_services/rsa_common/src/remote_interceptors_handler.c +++ b/bundles/remote_services/rsa_common/src/remote_interceptors_handler.c @@ -117,6 +117,7 @@ void remoteInterceptorsHandler_removeInterceptor(void *handle, void *svc, __attr entry_t *entry = arrayList_get(handler->interceptors, i); if (entry->interceptor == svc) { arrayList_remove(handler->interceptors, i); + free(entry); break; } } @@ -205,4 +206,4 @@ int referenceCompare(const void *a, const void *b) { long servRankingB = celix_properties_getAsLong(bEntry->properties, OSGI_FRAMEWORK_SERVICE_RANKING, 0); return celix_utils_compareServiceIdsAndRanking(servIdA, servRankingA, servIdB, servRankingB); -} \ No newline at end of file +} diff --git a/libs/dfi/gtest/src/json_rpc_tests.cpp b/libs/dfi/gtest/src/json_rpc_tests.cpp index ba83859cf..cc57b8faf 100644 --- a/libs/dfi/gtest/src/json_rpc_tests.cpp +++ b/libs/dfi/gtest/src/json_rpc_tests.cpp @@ -35,6 +35,7 @@ extern "C" { #include "dyn_type.h" #include "json_serializer.h" #include "json_rpc.h" +#include "celix_errno.h" static void stdLog(void*, int level, const char *file, int line, const char *msg, ...) { va_list ap; @@ -101,7 +102,7 @@ static void stdLog(void*, int level, const char *file, int line, const char *msg } int addFailed(void*, double , double , double *) { - return 0x02000001;// return customer error + return CELIX_CUSTOMER_ERROR_MAKE(0,1);// return customer error } int getName_example4(void*, char** result) { diff --git a/libs/utils/include/celix_errno.h b/libs/utils/include/celix_errno.h index d13c47747..9506a166d 100644 --- a/libs/utils/include/celix_errno.h +++ b/libs/utils/include/celix_errno.h @@ -77,7 +77,7 @@ const char* celix_strerror(celix_status_t status); * Customer error code mask * */ -#define CELIX_CUSTOMER_ERR_MASK 0x02000000 +#define CELIX_CUSTOMER_ERR_MASK 0x20000000 /*! * The facility of system error code, @@ -104,6 +104,12 @@ const char* celix_strerror(celix_status_t status); */ #define CELIX_ERROR_MAKE(fac,code) (((unsigned int)(fac)<<16) | ((code)&0xFFFF)) +/*! + * Make the customer error code + * \param usrFac Facility value of customer error code.It is defined by customer + * \param usrCode Code value of customer error codes.It is defined by customer + */ +#define CELIX_CUSTOMER_ERROR_MAKE(usrFac,usrCode) (CELIX_CUSTOMER_ERR_MASK | (((usrFac)&0x7FF)<<16) | ((usrCode)&0xFFFF)) /*! * Error code indicating successful execution of the function. From ab49295594012f6d2f6efeea396adc9f3770f022 Mon Sep 17 00:00:00 2001 From: xuzhenbao Date: Mon, 21 Feb 2022 16:54:44 +0800 Subject: [PATCH 08/11] remove redundant code --- .../gtest/src/rsa_client_server_tests.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/bundles/remote_services/remote_service_admin_dfi/gtest/src/rsa_client_server_tests.cc b/bundles/remote_services/remote_service_admin_dfi/gtest/src/rsa_client_server_tests.cc index 549ba1007..5afd956e3 100644 --- a/bundles/remote_services/remote_service_admin_dfi/gtest/src/rsa_client_server_tests.cc +++ b/bundles/remote_services/remote_service_admin_dfi/gtest/src/rsa_client_server_tests.cc @@ -143,7 +143,6 @@ typedef struct rsa_dfi_exception_test_service { celix_properties_t *svcInterceptorProps = celix_properties_create(); celix_properties_setLong(svcInterceptorProps, OSGI_FRAMEWORK_SERVICE_RANKING, 10); celix_service_registration_options_t svcInterceptorOpts{}; - memset(&svcInterceptorOpts,0,sizeof(svcInterceptorOpts)); svcInterceptorOpts.svc = serverSvcInterceptor; svcInterceptorOpts.serviceName = REMOTE_INTERCEPTOR_SERVICE_NAME; svcInterceptorOpts.serviceVersion = REMOTE_INTERCEPTOR_SERVICE_VERSION; @@ -160,7 +159,6 @@ typedef struct rsa_dfi_exception_test_service { celix_properties_t *clientInterceptorProps = celix_properties_create(); celix_properties_setLong(clientInterceptorProps, OSGI_FRAMEWORK_SERVICE_RANKING, 10); celix_service_registration_options_t clientInterceptorOpts{}; - memset(&clientInterceptorOpts,0,sizeof(clientInterceptorOpts)); clientInterceptorOpts.svc = clientSvcInterceptor; clientInterceptorOpts.serviceName = REMOTE_INTERCEPTOR_SERVICE_NAME; clientInterceptorOpts.serviceVersion = REMOTE_INTERCEPTOR_SERVICE_VERSION; From 64ebc78ca0737f3b5f871c3b63e83fcbdc0e75f0 Mon Sep 17 00:00:00 2001 From: xuzhenbao Date: Wed, 23 Feb 2022 15:23:30 +0800 Subject: [PATCH 09/11] change celix_errno.h according to code review --- libs/utils/include/celix_errno.h | 48 ++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/libs/utils/include/celix_errno.h b/libs/utils/include/celix_errno.h index 2132ebf0d..7c2b7c830 100644 --- a/libs/utils/include/celix_errno.h +++ b/libs/utils/include/celix_errno.h @@ -80,10 +80,10 @@ const char* celix_strerror(celix_status_t status); #define CELIX_CUSTOMER_ERR_MASK 0x20000000 /*! - * The facility of system error code, - * \note Error code 0 indicates success,it is not system error code. + * The facility of C errno, + * \note Error code 0 indicates success,it is not C errno. */ -#define CELIX_FACILITY_SYSTEM 0 +#define CELIX_FACILITY_CERRNO 0 /*! * The facility of celix default error code @@ -123,6 +123,7 @@ const char* celix_strerror(celix_status_t status); /*! * The range for Celix errors. + * \deprecated It is recommended to use facility indicate the range of error codes */ #define CELIX_ERRSPACE_SIZE 1000 @@ -132,30 +133,49 @@ const char* celix_strerror(celix_status_t status); */ #define CELIX_START_USERERR (CELIX_START_ERROR + CELIX_ERRSPACE_SIZE) +/*! + * @name celix default error codes + * @{ + */ + /*! * Exception indicating a problem with a bundle */ -#define CELIX_BUNDLE_EXCEPTION (CELIX_START_ERROR + 1) +#define CELIX_BUNDLE_EXCEPTION CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4465) + /*! * Invalid bundle context is used */ -#define CELIX_INVALID_BUNDLE_CONTEXT (CELIX_START_ERROR + 2) +#define CELIX_INVALID_BUNDLE_CONTEXT CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4466) /*! * Argument is not correct */ -#define CELIX_ILLEGAL_ARGUMENT (CELIX_START_ERROR + 3) -#define CELIX_INVALID_SYNTAX (CELIX_START_ERROR + 4) -#define CELIX_FRAMEWORK_SHUTDOWN (CELIX_START_ERROR + 5) -#define CELIX_ILLEGAL_STATE (CELIX_START_ERROR + 6) -#define CELIX_FRAMEWORK_EXCEPTION (CELIX_START_ERROR + 7) -#define CELIX_FILE_IO_EXCEPTION (CELIX_START_ERROR + 8) -#define CELIX_SERVICE_EXCEPTION (CELIX_START_ERROR + 9) +#define CELIX_ILLEGAL_ARGUMENT CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4467) +#define CELIX_INVALID_SYNTAX CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4468) +#define CELIX_FRAMEWORK_SHUTDOWN CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4469) +#define CELIX_ILLEGAL_STATE CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4470) +#define CELIX_FRAMEWORK_EXCEPTION CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4471) +#define CELIX_FILE_IO_EXCEPTION CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4472) +#define CELIX_SERVICE_EXCEPTION CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4473) /*! * Exception indicating a problem with a interceptor */ -#define CELIX_INTERCEPTOR_EXCEPTION (CELIX_START_ERROR + 10) +#define CELIX_INTERCEPTOR_EXCEPTION CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4474) + + +/*! @} *///celix default error codes + + +/*! + * @name C error map to celix + * @{ + */ + +#define CELIX_ENOMEM CELIX_ERROR_MAKE(CELIX_FACILITY_CERRNO,ENOMEM) + + +/*! @}*///C error map to celix -#define CELIX_ENOMEM ENOMEM /** * \} From b8a88633d5b3db5b7c7836a7e83aaca21eb20104 Mon Sep 17 00:00:00 2001 From: xuzhenbao Date: Wed, 23 Feb 2022 22:18:23 +0800 Subject: [PATCH 10/11] Convert jsonRpc Error to Celix default error --- .../src/export_registration_dfi.c | 3 ++- .../src/import_registration_dfi.c | 9 ++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.c b/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.c index 5acd02f2b..770ec59fd 100644 --- a/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.c +++ b/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.c @@ -168,7 +168,8 @@ celix_status_t exportRegistration_call(export_registration_t *export, char *data if (cont) { celixThreadMutex_lock(&export->mutex); if (export->active && export->service != NULL) { - status = jsonRpc_call(export->intf, export->service, data, &response); + int rc = jsonRpc_call(export->intf, export->service, data, &response); + status = (rc != 0) ? CELIX_BUNDLE_EXCEPTION : CELIX_SUCCESS; } else if (!export->active) { status = CELIX_ILLEGAL_STATE; celix_logHelper_warning(export->helper, "Cannot call an inactive service export"); diff --git a/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c b/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c index 1285162a9..e10997e6e 100644 --- a/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c +++ b/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c @@ -324,7 +324,8 @@ static void importRegistration_proxyFunc(void *userData, void *args[], void *ret char *invokeRequest = NULL; if (status == CELIX_SUCCESS) { - status = jsonRpc_prepareInvokeRequest(entry->dynFunc, entry->id, args, &invokeRequest); + int rc = jsonRpc_prepareInvokeRequest(entry->dynFunc, entry->id, args, &invokeRequest); + status = (rc != 0) ? CELIX_BUNDLE_EXCEPTION : CELIX_SUCCESS; //printf("Need to send following json '%s'\n", invokeRequest); } @@ -342,8 +343,10 @@ static void importRegistration_proxyFunc(void *userData, void *args[], void *ret if (status == CELIX_SUCCESS && rc == CELIX_SUCCESS && dynFunction_hasReturn(entry->dynFunc)) { //fjprintf("Handling reply '%s'\n", reply); int rsErrno = CELIX_SUCCESS; - status = jsonRpc_handleReply(entry->dynFunc, reply, args, &rsErrno); - if (rsErrno != CELIX_SUCCESS) { + int retVal = jsonRpc_handleReply(entry->dynFunc, reply, args, &rsErrno); + if(retVal != 0) { + status = CELIX_BUNDLE_EXCEPTION; + } else if (rsErrno != CELIX_SUCCESS) { //return the invocation error of remote service function *(int *) returnVal = rsErrno; } From a7472d13970c900e5f70846454822c1e2d87bc1f Mon Sep 17 00:00:00 2001 From: xuzhenbao Date: Fri, 25 Feb 2022 20:27:28 +0800 Subject: [PATCH 11/11] rename CELIX_FACILITY_NULL to CELIX_FACILITY_FRAMEWORK --- libs/utils/include/celix_errno.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/libs/utils/include/celix_errno.h b/libs/utils/include/celix_errno.h index 7c2b7c830..a1e76a1fe 100644 --- a/libs/utils/include/celix_errno.h +++ b/libs/utils/include/celix_errno.h @@ -89,7 +89,7 @@ const char* celix_strerror(celix_status_t status); * The facility of celix default error code * */ -#define CELIX_FACILITY_NULL 1 +#define CELIX_FACILITY_FRAMEWORK 1 /*! * The facility of the http suppoter error code @@ -141,26 +141,26 @@ const char* celix_strerror(celix_status_t status); /*! * Exception indicating a problem with a bundle */ -#define CELIX_BUNDLE_EXCEPTION CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4465) +#define CELIX_BUNDLE_EXCEPTION CELIX_ERROR_MAKE(CELIX_FACILITY_FRAMEWORK, 4465) /*! * Invalid bundle context is used */ -#define CELIX_INVALID_BUNDLE_CONTEXT CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4466) +#define CELIX_INVALID_BUNDLE_CONTEXT CELIX_ERROR_MAKE(CELIX_FACILITY_FRAMEWORK, 4466) /*! * Argument is not correct */ -#define CELIX_ILLEGAL_ARGUMENT CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4467) -#define CELIX_INVALID_SYNTAX CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4468) -#define CELIX_FRAMEWORK_SHUTDOWN CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4469) -#define CELIX_ILLEGAL_STATE CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4470) -#define CELIX_FRAMEWORK_EXCEPTION CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4471) -#define CELIX_FILE_IO_EXCEPTION CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4472) -#define CELIX_SERVICE_EXCEPTION CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4473) +#define CELIX_ILLEGAL_ARGUMENT CELIX_ERROR_MAKE(CELIX_FACILITY_FRAMEWORK, 4467) +#define CELIX_INVALID_SYNTAX CELIX_ERROR_MAKE(CELIX_FACILITY_FRAMEWORK, 4468) +#define CELIX_FRAMEWORK_SHUTDOWN CELIX_ERROR_MAKE(CELIX_FACILITY_FRAMEWORK, 4469) +#define CELIX_ILLEGAL_STATE CELIX_ERROR_MAKE(CELIX_FACILITY_FRAMEWORK, 4470) +#define CELIX_FRAMEWORK_EXCEPTION CELIX_ERROR_MAKE(CELIX_FACILITY_FRAMEWORK, 4471) +#define CELIX_FILE_IO_EXCEPTION CELIX_ERROR_MAKE(CELIX_FACILITY_FRAMEWORK, 4472) +#define CELIX_SERVICE_EXCEPTION CELIX_ERROR_MAKE(CELIX_FACILITY_FRAMEWORK, 4473) /*! * Exception indicating a problem with a interceptor */ -#define CELIX_INTERCEPTOR_EXCEPTION CELIX_ERROR_MAKE(CELIX_FACILITY_NULL, 4474) +#define CELIX_INTERCEPTOR_EXCEPTION CELIX_ERROR_MAKE(CELIX_FACILITY_FRAMEWORK, 4474) /*! @} *///celix default error codes