SendOperationResponse thread safety
Hi again. I just need some help clearing up some more fiber-related confusion.
Looking at MmoActor methods (like OperationSubscribeItem, for example), I see this a lot:
Where ExecItemOperation is:
This means that the MmoItem will be calling the peer's SendOperationResponse (and SendEvent, among other things) on the item's fiber. That doesn't matter if both the MmoActor and MmoItem are using the same fiber. But if that's the case, then what's the point of queuing the operation on the item's fiber? Why not just handle the whole operation in the operation handler instead of queueing the second half of the work?
In other words, it seems like this pattern is either not thread safe or needlessly confusing. But I'm pretty sure that I'm just not smart enough to see it correctly. Can anyone set me straight?
Looking at MmoActor methods (like OperationSubscribeItem, for example), I see this a lot:
item.Fiber.Enqueue(() => this.ExecItemOperation(() => this.ItemOperationSubscribeItem(item, operation, interestArea), sendParameters));
Where ExecItemOperation is:
private void ExecItemOperation(Func<OperationResponse> operation, SendParameters sendParameters) { OperationResponse result = operation(); if (result != null) { this.Peer.SendOperationResponse(result, sendParameters); } }
This means that the MmoItem will be calling the peer's SendOperationResponse (and SendEvent, among other things) on the item's fiber. That doesn't matter if both the MmoActor and MmoItem are using the same fiber. But if that's the case, then what's the point of queuing the operation on the item's fiber? Why not just handle the whole operation in the operation handler instead of queueing the second half of the work?
In other words, it seems like this pattern is either not thread safe or needlessly confusing. But I'm pretty sure that I'm just not smart enough to see it correctly. Can anyone set me straight?
0
Comments
-
It depends on what the operations are doing and *where* they do it.
If an operation affects a item, then the item should handle the operation and respond to avoid multiple threads changing the item at the same time.
The operation itself is passed on, so that is not affected by multiple threads either.
It might be overly complex in some cases but in general, we pass operations to the object that they affect to handle it there and only there.
Hope this helps (I'm not too deep into server code myself).0 -
Thanks, Tobias, I appreciate the response. That helps solidify my understanding of how things are supposed to work, and I agree with everything you said.
However, what you've described doesn't seem to match what's happening in the MmoDemo code that I referenced. I'll try to do a better job at explaining what I mean, and hopefully someone can tell me what I don't understand. Overall, my only goal is to gain an intuitive understanding of how Photon works, and how MmoDemo works, so that I can confidently implement the server-side code that we need in order to successfully ship our Photon game.
Specifically, it seems that MmoActor is passing an operation that operates on itself to be executed by an item on the item's fiber. This is the opposite of passing operations to the object that they affect. This is passing an operation that affects object A to be executed by object B. (unless I misunderstand the code)
1. item.Fiber.Enqueue( () => this.ExecItemOperation(
Enqueue a call to this.ExecItemOperation on item's fiber.
2. item.Fiber.Enqueue( () => this.ExecItemOperation( () => this.ItemOperationSubscribeItem(
Enqueue a call to this.ExecItemOperation on item's fiber, which will in turn call this.ItemOperationSubscribeItem on item's fiber.
3. (inside ExecItemOperation) this.Peer.SendOperationResponse(result, sendParameters);
From item's fiber, inside MmoActor.ItemOperationSubscribeItem, send an operation response by calling MmoActor.Peer.SendOperationResponse.
So it seems like at this point we can say that the object named "item" will be calling methods on instances of MmoActor and PeerBase from fiber A while those very same instances of MmoActor and PeerBase may be executing methods on themselves from fiber B (i.e. PeerBase.RequestFiber).
In other words, it seems like this could result in two separate thread simultaneously executing PeerBase.SendOperationResponse and MmoActor.ItemOperationSubscribeItem. As far as I can tell, those functions aren't thread-safe. So either I'm misunderstanding the code, or the code is incorrect. But considering that Photon seems to be pretty solidly built, I must be misunderstanding something.
In the case where all of these objects are using the same fiber, things will coincidentally work out OK since there's no possibility of simultaneous execution of enqueued actions. However, if the item and the actor are using different peers then simultaneous execution is possible and there doesn't seem to be any locking to protect against it.
But if the code is written to assume and require that all of the objects are using the same fiber, then there's no reason to split the operation across multiple fibers to begin with, and the code could be much cleaner.
I hope this rambling helps get my question across more clearly. Thanks again.0 -
I was into the code a bit when I noticed that my version is different from yours.
Which server SDK version are you using?0 -
Those code blocks come from ExitGames-Photon-Server-SDK_v3-0-15-2544-RC7.zip (the most recent version available), in src-server\Mmo\Photon.MmoDemo.Server\MmoActor.cs, on lines 755 and 1102.0
-
The code in this case doesn't look like multiple responses could be sent - the cases exclude each other.
But you are right: In some cases, the item's fiber will use peer.SendOperationResponse(). And it's possible to come up with situations where multiple threads use SendOperationResponse (by accident).
I think the mystery here is that SendOperationResponse is by itself thread safe and can be called from anywhere. The methods use the low level peer classes, which cope with locking internally. This is not described anywhere, which is the cause for this confusion. Sorry about that.However, if the item and the actor are using different peers then simultaneous execution is possible and there doesn't seem to be any locking to protect against it.
So, it does make sense to split tasks into threads by entity and have few locking in the higher levels of the logic but when it comes to just sending data to an actual client, you don't have to pass everything back into fibers of that peer, cause it handles this internally.0 -
SendOperationResponse is by itself thread safeif the item and the actor are using different peers
But in any case, after having another look at MmoActor.ItemOperationSubscribeItem with your comments in mind, I feel like I understand it now. It runs on the Item's fiber because interestArea.SubscribeItem requires it. The lock of interestArea.SyncRoot makes it safe. And all it does aside from that is call Peer.SendEvent, which is thread-safe.
Thanks!0 -
You're welcome! A good reason to take a look at some of the mmo codeI assume this means that all of Peer/PeerBase/ServerPeerBase is thread-safe?0
-
Sorry to resurrect this thread, but it would be very helpful if I had a list of which methods I can consider thread safe that so I don't have to worry about enqueuing calls to them them on fibers. Even if the answer is "assume nothing is safe", that's fine too. Thanks0
-
enqueuing on the fiber is always threadsafe if you enqueue it for the object you are operating on. Whats not safe is enqueuing on other objects fibers (room <-> room or actor <-> actor), for that you would need to send them a message so they can enqueue it themself0
-
All methods in the classes ServerPeerBase and PeerBase are thread safe.0