IDArling
IDArling copied to clipboard
IDArlingTeam
Implement collisions recovery
Seems that the IDA kernel will delete the type and then re-create it, causing the local_types_changed hook being called multiple times. When the network is quite good (no packet loss, very low delay), it doesn’t matter and works very well, but when I test it on a normal network(~2% loss) with 3 person, it seems that it will lead to racing conditions, corrupt the local types list. I tried to fix it with duplicate hook event detection, and the packet will be only sent once now, but still I doubt whether there is a better approach.
- Currently I send all the local types to the remote server, which will probably not that good when there are too much local types/network access is not stable
- I’m considering whether to post only the modified part of the local types, which won’t have the issues above. But as the IDA uses ordinal to reference the types, so there will be higher risk of inconsistency.
I’m in a dilemma again :(
I've tried adding a new local type and I see the hook being triggered 3 times:
[19:05:38][DEBUG] Sending packet: LocalTypesChangedEvent(local_type=[..., (u'\r\x01\x01', None, u'mystruct')])
[19:05:38][DEBUG] Sending packet: LocalTypesChangedEvent(local_type=[..., None])
[19:05:38][DEBUG] Sending packet: LocalTypesChangedEvent(local_type=[..., (u'\r\x19\x05\x05\x05', u'\x02a\x02b\x02c', u'mystruct')])
I think the best approach would be splitting the LocalTypesChanged events into 3 separate events: LocalTypeAdded, LocalTypeRemoved, LocalTypeChanged, each only applying to a specific index.
This would still require sending 3 (significantly smaller) packets, so I'm not sure if it would totally fix the issue or not, but I think it is worth a try at least. I will wait before merging your pull request.
However these events are all triggered by local_types_changed event, and I can't come up with any good solutions on how to distinguish them. You can try to changed a local type and you'll find the hook func will be called twice.
The key point is that the IDA internally refer the local types using ordinals, and we must maintain a list with same order across all clients, or the types in HexRays will be in chaos.
If we only send the changed part, I doubt that some other types will get overwritten with some slight desynchonrization.
PS: I don't think we need to sync the remove event, as the ordinal used by old type won't get reused when creating a new type.
I think I understand the issue at hand here, but by
If we only send the changed part, I doubt that some other types will get overwritten with some slight desynchonrization.
did you mean that you think that my approach will work or won't work?
Oh I mean both approach will fail on corner cases (e.g. bad network) But your approach won't be recovered that easily. For example: Before:
1 Type1
2 Type2
3 Type3
4 Type4
After 1 edit:
1 Type1
2 Type2
3 Type3'
4 Type4
If the edit operation on type wasn't correctly applied, then the Type3' will never be synced until another operation on it. With the situration above I thought we should send all structs every time.
Do you see the De-synchronization detected message when such an issue happens? If the answer is yes, then this isn't specific to local types. Is it a matter of what should we do when collisions happen.
Our current idea (that we haven't implemented) is to make all conflicting users download the last same of the IDB they were working on (auto-save is also not implemented yet) and send them all the event up to either the last event before the conflict, or the conflicting events but with their order corrected by the server.
Oh in this way I agree with you. For the three events you mentioned above, it seems that only one LocalTypesChanged event is also OK.
Could be confirm or infirm if you're seeing the message? If you do, we should rename this issue to something like "Implement collision recovery" because that's the underlying problem we need to tackle.
Reopening this issue to keep track of our needs on collisions recovery.
The current solution we've considered but not implemented yet is to to make all conflicting users download the last same of the IDB they were working on and send them all the events up to either the last event before the conflict, or the conflicting events but with their order corrected by the server.
Some resources related to this issue:
This is partially unrelated, but we could have a undo button with all these features, I think ?
