Skip to content

Fix for potential cyclical reference creation in RGAs#4

Open
LTibbetts wants to merge 2 commits intomasterfrom
LTibbetts-fix-rga-patching
Open

Fix for potential cyclical reference creation in RGAs#4
LTibbetts wants to merge 2 commits intomasterfrom
LTibbetts-fix-rga-patching

Conversation

@LTibbetts
Copy link
Copy Markdown

RON RGA applyPatch Bug

A bug in the RON RGA (Replicated Growable Array) implementation where HashMap.union bias in applyPatch causes interleaved patch vertices to become orphaned.

The Bug

In RON.Data.RGA.applyPatch (~line 298), the expression:

pure $ HashMap.insert parent item' targetItems <> newItems

uses HashMap.union (via <>), which is left-biased. targetItems contains stale itemNext pointers that override the correct pointers from newItems (the merge result). This causes vertices that should interleave into the linked list to become orphaned and silently dropped.

Fix — swap operands so newItems takes precedence:

pure $ HashMap.insert parent item' $ HashMap.union newItems targetItems

vertexListFromOps creates cycles from duplicate opIds

Location: ron/ron-rdt/lib/RON/Data/RGA.hs:135-143

vertexListFromOps :: [Vertex] -> Maybe VertexList
vertexListFromOps = foldr go mempty
  where
    go v@Op{opId} vlist = Just VertexList{listHead = opId, listItems = vlist'}
      where
        item itemNext = VertexListItem {itemValue = v, itemNext}
        vlist' = case vlist of
          Nothing -> HashMap.singleton opId (item Nothing)
          Just VertexList {listHead, listItems} ->
            HashMap.insert opId (item $ Just listHead) listItems
            -- ^^^^^^^^^^^^^^^^
            -- If opId already exists, HashMap.insert OVERWRITES the previous
            -- entry's next pointer, corrupting the linked-list structure

Effect: Duplicate opIds in the input [Op] list cause HashMap.insert to overwrite an existing vertex's itemNext, creating a cycle. This is the direct cause of the production crash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant