Please make sure you follow our codestyle rules. Travis fails because of that. You can see the result when you follow the "Details" link after the Travis message.
The main issue is that we use tabs instead of spaces to indent lines.
Also we use if clauses like this:

if (somescheck)
{
    some code
}

Okay:

• fixed "if" statements to meet the standards
• used tabs instead of spaces
• all comments now start with capital letter

Hope this will do

Missed (hopefully) one more thing... let's see

Travis is fine now. Thanks!
Ready for testing

First I would have thought that it would have been easier to simply wrap this in a transaction, but that was not correct since transactions wont help here.

But I think it is wrong that we don't support adding several rows with one query. I would rather fix that issue in our database driver instead of this rather complex change. Fixing that part would be the better fix in my view for this issue, since with one INSERT, we would not have this concurrency issue.

Did you happen on this bug specifically yourself, @jaworskimatt

@Hackwar we encountered this bug because multiple (dozens) Jomsocial customers were complaining about user groups getting randomly lost, with some of them the discussions got pretty heated because the bug was first noticed a few months ago. Since Jomsocial saves the user object a lot (don't ask me why, I don't know myself) we got these concurrency issues pretty often. It took us quite a while to track down what was going on (because first we looked for the problem inside Jomsocial itself).

I was thinking about a single insert and/or transactions, but I I am not sure how various database engines would handle them.

I think it is way better to read/compare and then insert/delete then delete everything and insert. Most of the time there isn't any change in the group member ship so nothing happened at all. Transactions or locking would be the best solution but this here is a step to make it better. Maybe the joomsocial guys should also check if they really need to change the user object every millisecond.

Please see #6065 for a better solution.

Hi
It seems the bug remains on J3.4. We just encountered it

Well, long forgotten thread.

I see this PR unsets the values from $this->groups that I think is not appropriate. Am I correct?