Only allowing a one specific request at a time

[moccha]

When a user submits a request to get an item, the server checks if they're allowed to get it and then rewards the item. However, I've noticed that if the client spams the request, for a short period of time the server constantly evaluates that they're allowed to receive the item and it will send multiple rewards until the database variable is altered, which can take half a second or more.

Is there a way to only evaluate the item request for that particular user and to wait until the request has finished processing, or is there an easier way? Thank you

[Lapo]

Hi,
yes it is a concurrency issue. The server uses multiple threads to service concurrent requests, so when the come in very fast they are handled in parallel.
When multiple threads call your methods you must make sure that data doesn't get corrupted.

Typically you will need to synchronize thread access at some point in the code. If you're working with simple numbers a "volatile" keyword in front of the field declaration will do the trick. If it's more complex data you will need to use a lock to essentially make access sequential, instead of parallel.

Here's an example:
http://howtodoinjava.com/core-java/mult ... d-example/

I would not recommend locking when accessing the database however. If you need locking at database level then I suggest you look into the documentation of your DB and activate it there, instead of doing it via code.

cheers

[moccha]

In my case, I am working with a simple RoomVar integer. If I use the volatile keyword, would the server block all other requests for this type while processing or just additional requests from the user?

My goal is to just block more than one request at a time from a single user, so if two users simultaneously requested the item, they would both receive a response and not be blocked since it's two requests coming from two different users. What are your thoughts, Lapo?

[Lapo]

I need to understand more clearly how this should work.
You wrote about a database... so I am no longer sure where the problem is.

Can you walk me through the request / response sequence between client and server and describe what issue you're actually having in detail?
Maybe also add some code snippets of the code that is not working as you'd expect.

thanks

[moccha]

Sure thing. Sorry for the confusion with a database, as I've actually changed it since then:

Client asks for an item via a simple command:

Code: Select all

requestItem(20);//Requesting item #20

Server receives requestItem command, checks that they have the required items before giving them the item:

Code: Select all

if(player.getVariable("item1").getIntValue() >= 50)//If player has required amount of materials, give them the requested item
{
     /* Item adding code is here, item1 is reset to "0" */
     getApi().setUserVariables(player, newItemData);//Update user
}

The server adds the item to the player's inventory and then deletes their required items. However, if the requestItem() command is sent rapidly, the server continually evaluates the "item1" value as greater than 50 for a short while before updating the UserVariable. I'd like it so that multiple users can send this request at once, but each user is only allowed one request to be evaluated at a time to avoid other requests being ignored.

[Lapo]

Thanks.

Question: since you're using a UserVariable (item1), is there anything preventing the client to cheat by changing value to that variable?
I mean, naturally your application doesn't allow that, but an attacker may hack the client and send a request to change that value (e.g setting it to a much higher value)

Back to your question you could fix the code like this:

Code: Select all

synchronized(player)
{
   if(player.getVariable("item1").getIntValue() >= 50)//If player has required amount of materials, give them the requested item
   {
        /* Item adding code is here, item1 is reset to "0" */
        getApi().setUserVariables(player, newItemData);//Update user
   }
}

This will make sure that if the same code is being executed by two threads, they will have to take turns instead of running in parallel and possibly corrupt the data.

hope it helps

[moccha]

Lapo, you scared me big time with that observation! As it turns out, I have set the Privilege Manager to disallow SetUserVar requests, but I greatly appreciate that you pointed that out as I wasn't even sure myself!!

Thank you very much for the code suggestion. I greatly value your help and support you've provided me. Have a great day.

EDIT: Since I never allow users to set their own variables, is there a way to automatically ban any user trying to set their own variable? That would be pretty nice!

[Lapo]

By disallowing the request there's no way somebody will be able to change UserVars from client side, but we don't provide a banning option for discarded requests.
If you really need that you could still allow the request, capture it via a request Filter and then insta-ban the client.
See the docs for filters here:
http://docs2x.smartfoxserver.com/Advanc ... er-filters

[moccha]

Sorry to bump this topic but I'd like to say 'thank you' again for the additional help.

[seeingrain]

I think for SFS framework, a better mechanism should be:
when there are multiple request for a same session in the queue, there should be only ONE worker thread to process them subsequently 1 by 1. Other thread cannot touch it even thread pool has idle thread.
to implement this, the message queue should be grouped by session ID, it's not a simple queue. with this implementation, extension programmer would not care about lock issues.