BUGFIX: segfault on error during subscribe, fixed deadlock
diff --git a/src/mod_netconf.c b/src/mod_netconf.c
index 8bc2435..2abc8b0 100644
--- a/src/mod_netconf.c
+++ b/src/mod_netconf.c
@@ -120,29 +120,7 @@
static pthread_key_t notif_history_key;
-static pthread_key_t err_reply_key;
-
-#define GETSPEC_ERR_REPLY json_object *err_reply = *((json_object **) pthread_getspecific(err_reply_key));
-
-#define CHECK_ERR_SET_REPLY \
-if (reply == NULL) { \
- GETSPEC_ERR_REPLY \
- if (err_reply != NULL) { \
- /* use filled err_reply from libnetconf's callback */ \
- reply = err_reply; \
- } \
-}
-
-#define CHECK_ERR_SET_REPLY_ERR(errmsg) \
-if (reply == NULL) { \
- GETSPEC_ERR_REPLY \
- if (err_reply == NULL) { \
- reply = create_error(errmsg); \
- } else { \
- /* use filled err_reply from libnetconf's callback */ \
- reply = err_reply; \
- } \
-}
+pthread_key_t err_reply_key;
volatile int isterminated = 0;
@@ -229,6 +207,10 @@
const char* sid)
{
json_object **err_reply_p = (json_object **) pthread_getspecific(err_reply_key);
+ if (err_reply_p == NULL) {
+ DEBUG("Error message was not allocated. %s", __func__);
+ return;
+ }
json_object *err_reply = *err_reply_p;
json_object *array = NULL;
@@ -321,6 +303,49 @@
}
+void create_err_reply_p()
+{
+ json_object **err_reply = calloc(1, sizeof(json_object **));
+ if (err_reply == NULL) {
+ DEBUG("Allocation of err_reply storage failed!");
+ return;
+ }
+ if (pthread_setspecific(err_reply_key, err_reply) != 0) {
+ DEBUG("cannot set thread-specific value.");
+ }
+}
+
+void clean_err_reply()
+{
+ json_object **err_reply = (json_object **) pthread_getspecific(err_reply_key);
+ if (err_reply != NULL) {
+ if (*err_reply != NULL) {
+ pthread_mutex_lock(&json_lock);
+ json_object_put(*err_reply);
+ pthread_mutex_unlock(&json_lock);
+ }
+ if (pthread_setspecific(err_reply_key, err_reply) != 0) {
+ DEBUG("Cannot set thread-specific hash value.");
+ }
+ }
+}
+
+void free_err_reply()
+{
+ json_object **err_reply = (json_object **) pthread_getspecific(err_reply_key);
+ if (err_reply != NULL) {
+ if (*err_reply != NULL) {
+ pthread_mutex_lock(&json_lock);
+ json_object_put(*err_reply);
+ pthread_mutex_unlock(&json_lock);
+ }
+ free(err_reply);
+ err_reply = NULL;
+ if (pthread_setspecific(err_reply_key, err_reply) != 0) {
+ DEBUG("Cannot set thread-specific hash value.");
+ }
+ }
+}
/**
* \defgroup netconf_operations NETCONF operations
@@ -1751,14 +1776,8 @@
char *buffer = NULL;
/* init thread specific err_reply memory */
- json_object **err_reply = calloc(1, sizeof(json_object **));
- if (err_reply == NULL) {
- DEBUG("Allocation of err_reply storage failed!");
- isterminated = 1;
- }
- if (pthread_setspecific(err_reply_key, err_reply) != 0) {
- DEBUG("notif_history: cannot set thread-specific hash value.");
- }
+ create_err_reply_p();
+
while (!isterminated) {
fds.fd = client;
fds.events = POLLIN;
@@ -1829,7 +1848,7 @@
}
/* null global JSON error-reply */
- (*err_reply) = NULL;
+ clean_err_reply();
/* prepare reply envelope */
if (reply != NULL) {
@@ -1895,17 +1914,12 @@
break;
}
+ CHECK_ERR_SET_REPLY
if (reply == NULL) {
- if (*err_reply == NULL) {
- /** \todo more clever error message wanted */
- pthread_mutex_lock(&json_lock);
- reply = json_object_new_object();
- json_object_object_add(reply, "type", json_object_new_int(REPLY_OK));
- pthread_mutex_unlock(&json_lock);
- } else {
- /* use filled err_reply from libnetconf's callback */
- reply = *err_reply;
- }
+ pthread_mutex_lock(&json_lock);
+ reply = json_object_new_object();
+ json_object_object_add(reply, "type", json_object_new_int(REPLY_OK));
+ pthread_mutex_unlock(&json_lock);
}
break;
case MSG_KILL:
@@ -1963,35 +1977,18 @@
free(buffer);
buffer = NULL;
}
- if ((err_reply != NULL) && (*err_reply != NULL)) {
- json_object_put(*err_reply);
- *err_reply = NULL;
- }
pthread_mutex_unlock(&json_lock);
+ clean_err_reply();
} else {
pthread_mutex_unlock(&json_lock);
DEBUG("Reply is NULL, shouldn't be...");
- if (*err_reply == NULL) {
- DEBUG("No error was set - really strange situation");
- } else {
- DEBUG("Error was set but it was not sent.");
- }
continue;
}
}
}
free (arg);
- err_reply = (json_object **) pthread_getspecific(err_reply_key);
- if (err_reply != NULL) {
- if (*err_reply != NULL) {
- pthread_mutex_lock(&json_lock);
- json_object_put(*err_reply);
- pthread_mutex_unlock(&json_lock);
- }
- free(err_reply);
- err_reply = NULL;
- }
+ free_err_reply();
return retval;
}
diff --git a/src/mod_netconf.h b/src/mod_netconf.h
index bd497a7..2fcea4a 100644
--- a/src/mod_netconf.h
+++ b/src/mod_netconf.h
@@ -84,6 +84,9 @@
extern pthread_rwlock_t session_lock; /**< mutex protecting netconf_session_list from multiple access errors */
+extern pthread_key_t err_reply_key;
+extern pthread_mutex_t json_lock;
+
json_object *create_error(const char *errmess);
json_object *create_ok();
@@ -102,5 +105,32 @@
} \
} while (0);
+#define GETSPEC_ERR_REPLY \
+json_object **err_reply_p = (json_object **) pthread_getspecific(err_reply_key); \
+json_object *err_reply = ((err_reply_p != NULL)?(*err_reply_p):NULL);
+
+#define CHECK_ERR_SET_REPLY \
+if (reply == NULL) { \
+ GETSPEC_ERR_REPLY \
+ if (err_reply != NULL) { \
+ /* use filled err_reply from libnetconf's callback */ \
+ reply = err_reply; \
+ } \
+}
+
+#define CHECK_ERR_SET_REPLY_ERR(errmsg) \
+if (reply == NULL) { \
+ GETSPEC_ERR_REPLY \
+ if (err_reply == NULL) { \
+ reply = create_error(errmsg); \
+ } else { \
+ /* use filled err_reply from libnetconf's callback */ \
+ reply = err_reply; \
+ } \
+}
+void create_err_reply_p();
+void clean_err_reply();
+void free_err_reply();
+
#endif
diff --git a/src/notification-server.c b/src/notification-server.c
index fbab20d..7c2d8f0 100644
--- a/src/notification-server.c
+++ b/src/notification-server.c
@@ -562,11 +562,19 @@
}
DEBUG("Send NC subscribe.");
- /** \todo replace with sth like netconf_op(http_server, session_hash, rpc) */
+ create_err_reply_p();
if (send_recv_process(session, "subscribe", rpc) != 0) {
DEBUG("Subscription RPC failed.");
goto operation_failed;
}
+
+ GETSPEC_ERR_REPLY
+ if (err_reply != NULL) {
+ free_err_reply();
+ DEBUG("RPC-Error received and cleaned, because we can't send it anywhere.");
+ goto operation_failed;
+ }
+
rpc = NULL; /* just note that rpc is already freed by send_recv_process() */
locked_session->ntfc_subscribed = 1;
@@ -658,9 +666,11 @@
while ((notif = (notification_t *) apr_array_pop(ls->notifications)) != NULL) {
n = 0;
+ pthread_mutex_lock(&json_lock);
json_object *notif_json = json_object_new_object();
json_object_object_add(notif_json, "eventtime", json_object_new_int64(notif->eventtime));
json_object_object_add(notif_json, "content", json_object_new_string(notif->content));
+ pthread_mutex_unlock(&json_lock);
const char *msgtext = json_object_to_json_string(notif_json);
@@ -673,7 +683,9 @@
break;
}
+ pthread_mutex_lock(&json_lock);
json_object_put(notif_json);
+ pthread_mutex_unlock(&json_lock);
free(notif->content);
}
DEBUG("notification: POP notifications done");