Bug! If the number of sent SMS exceeds the current balance, the balance goes into minus

playSMS Help
1

Anton, Huy!
I noticed one unpleasant feature: In cases where the number of sent SMS is more than the balance, for example, when there is 1000 SMS in the user’s balance and he wants to send 3000, SMS sending does not stop at 1000 but continues to go in minutes and reaches -2000. Even the “Ban” of the user does not help.

P.S. PlaySMS version: 1.4.3

Yours faithfully,
Jamshid Tursnuov

2

So, how to re-produce is something like this:

  1. Set user’s credit to 1000
  2. Set rate to 1 credit per SMS
  3. Send 3000 SMS

How do you send 3000 SMS in this case ?

anton

3

Anton didn’t work, tried it in 2 versions and got the message in both:
In the first case, I tried to form a message with less than 160 characters and using Send from file. There were 0.02 units on the user’s balance (each SMS is equal to 0.01), I sent a file with 4 numbers, it didn’t work.
In the second case, I tried to compose an SMS from UNICODE characters and indicate more than 80 characters (so that each SMS contains a total of 2 SMS), and in this case the system responded correctly.

In the first case, I tried to form a message with less than 160 characters and using Send from file. There were 0.02 units on the user’s balance (each SMS is equal to 0.01), I sent a file with 4 numbers, it didn’t work.
In the second case, I tried to compose an SMS from UNICODE characters and indicate more than 80 characters (so that each SMS contains a total of 2 SMS), and in this case the system responded correctly.

In both cases I received:
failed user do not have enough credit.

But sometimes it happens, namely, I see that the user’s balance has gone into negative territory, PlaySMS has already sent all messages to the gateway and its balance will constantly go into negative territory. We have to delete messages in Redis, which helps.

Yours faithfully,
Jamshid Tursunov

4

I can confirm it happens. I’ll fix it.

I just tested 2000 SMS with the same message, thus having only 1 queue, playSMS successfully blocking this because calculating Balance done before processing any SMS.

But if you send 2000 SMS with different message, thus having 2000 queues in one go, playSMS will process this and balance will be negative. This is because playSMS counting current balance with that per queue, which is under current balance.

So, counting credit/balance is correct, but getting into negative balance is not OK. playSMS need to calculate everything before processing SMS in sendsms(), and in sendsmsd() there must be a check with current realtime balance.

anton

5

Excellent thank you.
Good luck and all the best to you.
Thank you for your product.

Yours faithfully,
Jamshid Tursunov

6

Hmm, actually my tests just finished.

I set credit to 1000 and rate to 1. I sent 2000 SMS with each message different, so 2000 queues created and processed.

By the time user’s credit reached -1, the queue stopped, so it was sent to only 1000 SMS and the rest of the SMS set to failed.

It works as expected tho, I meant the negative is minimal, or at least acceptable with current solution.

Let me test again.

anton

7

I can see in my tests thats what happened.

Latest test:

  • Add user1
  • Add credit 100 to user1
  • Set rate to 1 credit/SMS
  • user1 send from file to 200 destinations with different message each

Results:

  1. playSMS passing the send from file to sendsms() eventhough the submission will take 200 credits where user1 only have 100 credits
  2. playSMS processing more than the limit of credit, in my test was delivered to 103 SMS
  3. 103 SMS delivered, and 97 failed due to insufficient balance for user1
  4. There is 1 SMS not billed, since the last balance was -2, then it should be delievered to 102 instead of 103 destinations

Things needed a fix:

  1. Via send from file or other means of submitting SMS playSMS needs to check againt user’s balance before handing over to sendsms()
  2. Update sendsmsd() logic’s to reduce the negative, even its only -1 or -2
  3. There was 1 SMS, always 1 SMS, that is not billed correctly, in last test it should be delivered to 102 SMS instead of 103

anton

8

The issue is due to race condition in reading the adhoc credit. The race condition happened because of multiple playSMS daemon reading database. Adhoc credit is temporary credit/balance used by simplerate_hook_rate_cansend() to decide whether the user can continue processing the SMS for delivery or not.

Set this to 1 in web/config.php then calculation should be correct:

// limit the number of queue processed by sendsmsd in one time
$core_config['sendsmsd_queue'] = 1;

But it will slowdown sms processing of course.

anton

9

In master version I added a check for user’s balance before processing uploaded SMS. I haven’t test it in 1.4.3, but maybe you can just drop replace your feature/sendfromfile/*.php files with this:

You also need to update DB table playsms_featureSendfromfile using this SQL:

The DB update won’t interfere with sendfromfile from 1.4.3, so no need to revert if the test fails.

anton

10

Hi, Anton!
Thank you for trying to make PlaySMS better.
As for the option to send SMS using “Send from file”, I did not observe such system behavior there.
The user’s balance goes into the negative when working with the API, and such a case has repeatedly happened.

Yours faithfully,
Jamshid Tursunov

11

Yes, one step at a time, theres a roadblock in fixing this issue, namely it needs trasactions or some lock, or better logic, when multiple playSMS daemon reading/updating current adhoc balance that is used to allow/restrict SMS processing.

I probably can do the same for now on webservices, to check againts balance before actually process submitted SMS. But that’s not fixing the actual issue.

anton

12

Thank you Anton!

Yours faithfully,
Jamshid Tursunov

13

This topic was automatically closed 60 days after the last reply. New replies are no longer allowed.