RE: Does opensmppbox and smsbox have serious memory issues?

[Rene Kluwen]

Hello Andreas,

 

The smsc_id that is returned by boxc_route_msg_to_smsc is NOT a copy, so it
shouldn't be destroyed afterwards.

But you put the finger on the right spot. There was a memory leak in that
function.

 

I commited attached diff to svn. Please, can somebody test it and see if
this makes _the_ difference for opensmppbox?

 

== Rene

 

 

From: Andreas Fink [mailto:af...@list.fink.org] 
Sent: maandag 28 april 2014 12:51
To: Rene Kluwen
Cc: hbil...@ecommunicate.biz; devel Devel; Hanh Le Bich
Subject: Re: Does opensmppbox and smsbox have serious memory issues?

 

 





==31087== 4,986,528 (77,472 direct, 4,909,056 indirect) bytes in 4,842
blocks are definitely lost in loss record 813 of 813
==31087==    at 0x4027434: malloc (vg_replace_malloc.c:291)
==31087==    by 0x80970B3: gw_native_malloc (gwmem-native.c:87)
==31087==    by 0x80A37A1: octstr_create_from_data_real (octstr.c:263)
==31087==    by 0x80A3916: octstr_create_real (octstr.c:245)
==31087==    by 0x80A908E: octstr_format_valist_real (octstr.c:2480)
==31087==    by 0x80A9366: octstr_format (octstr.c:2469)
==31087==    by 0x80534F5: boxc_route_msg_to_smsc (opensmppbox.c:1791)
==31087==    by 0x8057AAE: smpp_to_bearerbox (opensmppbox.c:1638)
==31087==    by 0x80983AE: new_thread (gwthread-pthread.c:385)
==31087==    by 0x46F9C38: start_thread (pthread_create.c:304)
==31087==    by 0x482F78D: clone (clone.S:130)

 

this seems to be a leak in opensmppbox in this code segment (handle_pdu) in
opensmppbox.c around line 1634

 

 

            case submit_sm:
                        msg = pdu_to_msg(box, pdu, &reason);
                        msg2 = msg;
                        if (msg == NULL) {
                                   resp = smpp_pdu_create(generic_nack,
pdu->u.submit_sm.sequence_number);
                                   resp->u.generic_nack.command_status =
SMPP_ESME_RUNKNOWNERR;
                        }
                        else {
                                   Octstr *smsc_id =
boxc_route_msg_to_smsc(box, msg);  <-smsc_id is allocated but never freeed.
                                   check_multipart(box, msg, &msg_to_send,
&msg2, &parts_list);
                                   msg->sms.smsc_id = smsc_id ?
octstr_duplicate(smsc_id) : NULL; /* its duplicated here... */
                                   msg->sms.boxc_id =
octstr_duplicate(box->boxc_id);
                                   msg_dump(msg, 0);
                                   resp = smpp_pdu_create(submit_sm_resp,
pdu->u.submit_sm.sequence_number);
                                   msgid = generate_smppid(msg);
                                   msg->sms.dlr_url =
octstr_duplicate(msgid);
                                   resp->u.submit_sm_resp.message_id =
msgid;
                                   if (msg_to_send) {
                                               if
(DLR_IS_ENABLED(msg2->sms.dlr_mask)) {
                                                           hold_service =
msg2->sms.service;
                                                           msg2->sms.service
= octstr_format("%ld", msg2->sms.time);
                                                           msgid =
generate_smppid(msg2);
                                                           if (parts_list) {
 
msg2->sms.dlr_url = concat_msgids(msgid, parts_list);
                                                           }
 
dlr_add(box->boxc_id, msgid, msg2);
 
octstr_destroy(msgid);
 
octstr_destroy(msg2->sms.service);
                                                           msg2->sms.service
= hold_service;
                                               }
                                               uuid_unparse(msg2->sms.id,
id);
                                               msgid = octstr_create(id);
                                               dict_put(box->msg_acks,
msgid, resp);
                                               octstr_destroy(msgid);
                                               resp = NULL;
 
send_msg(box->bearerbox_connection, box, msg2);
                                               if (parts_list) {
                                                           /* destroy values
*/
 
gwlist_destroy(parts_list, msg_destroy_item);
                                               }
                                   }

                                   octstr_destroy(smsc_id); /* fix for this
leak */
                        }
                        break;

 

This is a leak which would leak a few bytes per SMS.

 

 

leak.diff
Description: Binary data