Re: [PATCH]: HTTP reply from sendota fixes (#1)
"Dziugas Baltrunas" <[email protected]>
| Newsgroups | gmane.comp.mobile.kannel.devel |
|---|---|
| Message-ID | <[email protected]> |
Hi, attached is an attempt to follow Alexander's advice long time ago for Kalle to put client into dict before sending a message to bearerbox. Not sure if it will work in all cases, so additional testing is required. Patch also includes fix for freeing msg which I sent before. To be more precise, I'm not sure if dict_remove(client_dict, stored_uuid) in my patch is necessary since smsbox.c:send_message() _never_ returns -1, although it's stated so and there are such checks in the code. This becomes more obvious when we look to write_to_bearerbox(), which returns void, so we actually never really know, whether bearerbox accepted the message or not. Thanks. On 3/21/06, Dziugas Baltrunas <[email protected]> wrote: > Hi, list, > > attached patch fixes freeing of msg structure in smsbox.c sendota > stuff before extracting it's uuid with store_uuid(). Without patch > applied you can see the following in the logs: > > 2006-03-20 18:35:36 [29721] [3] DEBUG: Stored UUID > 00000000-0000-0000-0000-000000000000 > > However, current patch does not solve HTTP reply from sendota > completely, since it's related how fast we get an ACK from bearerbox. > If we're successful, we'll get "0: Accepted for delivery", and if not, > no answer will be sent at all. This can be illustrated by these > snippets: > > 1. We got an ACK from bearerbox (read_messages_from_bearerbox()) > before store_uuid() was called: > > 2006-03-21 12:47:37 [19015] [3] INFO: /cgi-bin/sendota <<default>> <XXX> > 2006-03-21 12:47:37 [19015] [3] DEBUG: message length 446, sending 4 messages > 2006-03-21 12:47:37 [19015] [0] DEBUG: Got ACK (0) of > 8b37ca04-896f-4510-8d2d-000289b6b8b2 > 2006-03-21 12:47:37 [19015] [0] DEBUG: No client - multi-send or ACK > to pull-reply > 2006-03-21 12:47:37 [19015] [3] DEBUG: Stored UUID > 8b37ca04-896f-4510-8d2d-000289b6b8b2 > 2006-03-21 12:47:37 [19015] [3] DEBUG: Status: 202 Answer: <Sent.> > 2006-03-21 12:47:37 [19015] [3] DEBUG: Delayed reply - wait for bearerbox > > 2. We got an ACK from bearerbox after store_uuid() was called (as expected): > > 2006-03-21 12:47:09 [19015] [3] INFO: /cgi-bin/sendota <<default>> <XXX> > 2006-03-21 12:47:09 [19015] [3] DEBUG: message length 446, sending 4 messages > 2006-03-21 12:47:09 [19015] [3] DEBUG: Stored UUID > f6b0cb63-b561-49fa-951b-09aa5177067e > 2006-03-21 12:47:09 [19015] [3] DEBUG: Status: 202 Answer: <Sent.> > 2006-03-21 12:47:09 [19015] [3] DEBUG: Delayed reply - wait for bearerbox > 2006-03-21 12:47:09 [19015] [0] DEBUG: Got ACK (0) of > f6b0cb63-b561-49fa-951b-09aa5177067e > > This is relevant only when immediate_sendsms_reply is false (which is > default!), so I should ask Kalle, the author of introducing this > feature, comment on possible thread synchronisation issues, which IMHO > sometimes could also arise for ordinary sendsms request. > > Thanks, > Dziugas > > > -- Dziugas
immediate_sendsms_reply_ver2.patch
(text/plain, 7.8 KB)
Index: smsbox.c
===================================================================
RCS file: /home/cvs/gateway/gw/smsbox.c,v
retrieving revision 1.260
diff -u -r1.260 smsbox.c
--- smsbox.c 9 Dec 2005 02:14:31 -0000 1.260
+++ smsbox.c 21 Mar 2006 13:09:33 -0000
@@ -1958,7 +1958,7 @@
static Octstr *smsbox_req_handle(URLTranslation *t, Octstr *client_ip,
- Octstr **stored_uuid,
+ HTTPClient *client,
Octstr *from, Octstr *to, Octstr *text,
Octstr *charset, Octstr *udh, Octstr *smsc,
int mclass, int mwi, int coding, int compress,
@@ -1969,6 +1969,7 @@
{
Msg *msg = NULL;
Octstr *newfrom, *returnerror, *receiv;
+ Octstr *stored_uuid = NULL;
List *failed_id, *allowed, *denied;
int no_recv, ret = 0, i;
long del;
@@ -2292,6 +2293,11 @@
*/
failed_id = gwlist_create();
+ if (!immediate_sendsms_reply) {
+ stored_uuid = store_uuid(msg);
+ dict_put(client_dict, stored_uuid, client);
+ }
+
while ((receiv = gwlist_extract_first(allowed)) != NULL) {
O_DESTROY(msg->sms.receiver);
@@ -2313,13 +2319,6 @@
udh == NULL ? ( text == NULL ? "" : octstr_get_cstr(text) ) : "<< UDH >>");
}
}
- /* Store id if needed for a delayed HTTP reply */
-
- if (!immediate_sendsms_reply) {
- *stored_uuid = store_uuid(msg);
- }
-
-
msg_destroy(msg);
gwlist_destroy(receiver, octstr_destroy_item);
@@ -2377,6 +2376,8 @@
octstr_destroy(from);
*status = HTTP_INTERNAL_SERVER_ERROR;
returnerror = octstr_create("Sending failed.");
+ if (!immediate_sendsms_reply)
+ dict_remove(client_dict, stored_uuid);
/*
* Append all receivers to the returned body in case this is
@@ -2389,6 +2390,8 @@
}
}
+ if (stored_uuid)
+ octstr_destroy(stored_uuid);
octstr_destroy(receiv);
gwlist_destroy(failed_id, octstr_destroy_item);
gwlist_destroy(denied, octstr_destroy_item);
@@ -2470,7 +2473,7 @@
* Args: args contains the CGI parameters
*/
static Octstr *smsbox_req_sendsms(List *args, Octstr *client_ip, int *status,
- Octstr **stored_uuid)
+ HTTPClient *client)
{
URLTranslation *t = NULL;
Octstr *tmp_string;
@@ -2568,7 +2571,7 @@
return octstr_create("Empty receiver number not allowed, rejected");
}
- return smsbox_req_handle(t, client_ip, stored_uuid, from, to, text, charset, udh,
+ return smsbox_req_handle(t, client_ip, client, from, to, text, charset, udh,
smsc, mclass, mwi, coding, compress, validity,
deferred, status, dlr_mask, dlr_url, account,
pid, alt_dcs, rpi, NULL, binfo, priority);
@@ -2582,7 +2585,7 @@
*/
static Octstr *smsbox_sendsms_post(List *headers, Octstr *body,
Octstr *client_ip, int *status,
- Octstr **stored_uuid)
+ HTTPClient *client)
{
URLTranslation *t = NULL;
Octstr *user, *pass, *ret, *type;
@@ -2678,7 +2681,7 @@
}
if (ret == NULL)
- ret = smsbox_req_handle(t, client_ip, stored_uuid, from, to, body, charset,
+ ret = smsbox_req_handle(t, client_ip, client, from, to, body, charset,
udh, smsc, mclass, mwi, coding, compress,
validity, deferred, status, dlr_mask,
dlr_url, account, pid, alt_dcs, rpi, tolist,
@@ -2789,10 +2792,12 @@
* Args: list contains the CGI parameters
*/
static Octstr *smsbox_req_sendota(List *list, Octstr *client_ip, int *status,
- Octstr **stored_uuid)
+ HTTPClient *client)
{
Octstr *id, *from, *phonenumber, *smsc, *ota_doc, *doc_type, *account;
CfgGroup *grp;
+ Octstr *returnerror;
+ Octstr *stored_uuid = NULL;
List *grplist;
Octstr *p;
URLTranslation *t;
@@ -2944,21 +2949,29 @@
info(0, "%s <%s> <%s>", octstr_get_cstr(sendota_url),
id ? octstr_get_cstr(id) : "<default>", octstr_get_cstr(phonenumber));
+ if (!immediate_sendsms_reply) {
+ stored_uuid = store_uuid(msg);
+ dict_put(client_dict, stored_uuid, client);
+ }
+
ret = send_message(t, msg);
- msg_destroy(msg);
if (ret == -1) {
error(0, "sendota_request: failed");
*status = HTTP_INTERNAL_SERVER_ERROR;
- return octstr_create("Sending failed.");
- }
- else if (!immediate_sendsms_reply) {
- *stored_uuid = store_uuid(msg);
+ returnerror = octstr_create("Sending failed.");
+ dict_remove(client_dict, stored_uuid);
+ } else {
+ *status = HTTP_ACCEPTED;
+ returnerror = octstr_create("Sent.");
}
-
- *status = HTTP_ACCEPTED;
- return octstr_create("Sent.");
+ msg_destroy(msg);
+
+ if (stored_uuid)
+ octstr_destroy(stored_uuid);
+
+ return returnerror;
}
@@ -2972,11 +2985,12 @@
*/
static Octstr *smsbox_sendota_post(List *headers, Octstr *body,
Octstr *client_ip, int *status,
- Octstr **stored_uuid)
+ HTTPClient *client)
{
Octstr *name, *val, *ret;
Octstr *from, *to, *id, *user, *pass, *smsc;
Octstr *type, *charset, *doc_type, *ota_doc, *sec, *pin;
+ Octstr *stored_uuid = NULL;
URLTranslation *t;
Msg *msg;
long l;
@@ -3125,20 +3139,30 @@
info(0, "%s <%s> <%s>", octstr_get_cstr(sendota_url),
id ? octstr_get_cstr(id) : "XML", octstr_get_cstr(to));
+
+ if (!immediate_sendsms_reply) {
+ stored_uuid = store_uuid(msg);
+ dict_put(client_dict, stored_uuid, client);
+ }
+
r = send_message(t, msg);
- msg_destroy(msg);
if (r == -1) {
- error(0, "sendota_request: failed");
- *status = HTTP_INTERNAL_SERVER_ERROR;
- ret = octstr_create("Sending failed.");
- }
- else if (!immediate_sendsms_reply) {
- *stored_uuid = store_uuid(msg);
+ error(0, "sendota_request: failed");
+ *status = HTTP_INTERNAL_SERVER_ERROR;
+ ret = octstr_create("Sending failed.");
+ if (!immediate_sendsms_reply)
+ dict_remove(client_dict, stored_uuid);
+ } else {
+ *status = HTTP_ACCEPTED;
+ ret = octstr_create("Sent.");
}
- *status = HTTP_ACCEPTED;
- ret = octstr_create("Sent.");
+ msg_destroy(msg);
+
+ if (stored_uuid)
+ octstr_destroy(stored_uuid);
+
}
}
@@ -3157,9 +3181,6 @@
Octstr *ip, *url, *body, *answer;
List *hdrs, *args;
int status;
- Octstr *stored_uuid;
-
- stored_uuid = NULL;
for (;;) {
client = http_accept_request(sendsms_port, &ip, &url, &hdrs, &body,
@@ -3183,9 +3204,9 @@
* related routine handle the checking
*/
if (body == NULL)
- answer = smsbox_req_sendsms(args, ip, &status, &stored_uuid);
+ answer = smsbox_req_sendsms(args, ip, &status, client);
else
- answer = smsbox_sendsms_post(hdrs, body, ip, &status, &stored_uuid);
+ answer = smsbox_sendsms_post(hdrs, body, ip, &status, client);
}
/* XML-RPC */
else if (octstr_compare(url, xmlrpc_url) == 0)
@@ -3203,9 +3224,9 @@
else if (octstr_compare(url, sendota_url) == 0)
{
if (body == NULL)
- answer = smsbox_req_sendota(args, ip, &status, &stored_uuid);
+ answer = smsbox_req_sendota(args, ip, &status, client);
else
- answer = smsbox_sendota_post(hdrs, body, ip, &status, &stored_uuid);
+ answer = smsbox_sendota_post(hdrs, body, ip, &status, client);
}
/* add aditional URI compares here */
else {
@@ -3222,12 +3243,10 @@
octstr_destroy(body);
http_destroy_cgiargs(args);
- if (immediate_sendsms_reply || status != HTTP_ACCEPTED || stored_uuid == NULL)
+ if (immediate_sendsms_reply || status != HTTP_ACCEPTED)
http_send_reply(client, status, sendsms_reply_hdrs, answer);
else {
debug("sms.http", 0, "Delayed reply - wait for bearerbox");
- dict_put(client_dict, stored_uuid, client);
- octstr_destroy(stored_uuid);
}
octstr_destroy(answer);
}