Hi,
I kept this in mind, but I was trying to follow current indenting of
the relevant blocks which is already messed quite a lot, otherwise
patch would became irrelevant for the fix, but indents.
Anyway, fixed some of them.
On 3/23/06, Alexander Malysh <[EMAIL PROTECTED]> wrote:
Hi,
patch looks good. one thing: please fix coding style (indents).
Otherwise I'm +1 for it.
Thanks,
Alex
Dziugas Baltrunas wrote:
Hi again,
any objections on this? I've already tested the patch on bigger loads
and it seems to work fine.
Sometimes you can see "Delayed reply - wait for bearerbox" after " Got
ACK" in the logs, but this is obvious since we print the message
before puting client into dict.
On 3/21/06, Dziugas Baltrunas <[EMAIL PROTECTED]> wrote:
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
--
Dziugas
--
Thanks,
Alex
--
Dziugas
------------------------------------------------------------------------
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);
}
