6269bab in the gnx branch: a fix for #1437

[Edward K. Ream]

This post discusses a possible fix for #1437: Leo-Bridge can be wrong about an outline read from a file.

This is a tricky topic, so I want to walk myself, Bob, and other interested parties through the problem and the fix.

In the first comment of #1437 Bob says:

"Before reading a file, Leo generates one or more nodes with GNX's for the file. Usually this is harmless because the GNX's generated do not match any of the GNX's in the outline being read. But when a generated GNX equals a GNX in the outline being read, Leo is misled about the outline contents."

Restating the problem

LeoFrame.createFirstTreeNode allocates a vnode, c.hiddenRootNode, before reading the file. That means that there is no way to ensure that c.hiddenRootNode.gnx won't conflict with the vnodes in the file that is about to be read.

This potential clash only happens when the bridge opens two or more .leo files within a second or so.

leoBridge.py typically opens .leo files using g.openWithFileName. The call chain is long and complex, so adding kwargs along the way is out of the question.

The fix

Rev 6269bab in the gnx branch follows Bob's third solution, more or less. Here are the details:

1. frame.createFirstTreeNode now sets c.hiddenRootNode.gnx to a dummy value, but only when g.app.inBridge is True:

def createFirstTreeNode(self):
    """Create a new vnode as the root of the to-be-created tree."""
    c = self.c
    # #1437: Use a dummy gnx in the bridge.
    gnx = '**dummy-gnx**' if g.app.inBridge else None
    v = leoNodes.VNode(context=c, gnx=gnx)
    p = leoNodes.Position(v)
    v.initHeadString("NewHeadline")
    # p.moveToRoot would be wrong: the node hasn't been linked yet.
    p._linkAsRoot(oldRoot=None)

The guard on g.app.inBridge ensures that only the bridge can ever be affected by the new code.

2. After reading the file, bridge.openLeoFile sets c.hiddenRootNode.gnx to a value that doesn't conflict with any gnx in the just-read file:

# Existing code: make sure all new gnx's are unique.
g.app.nodeIndices.compute_last_index(c)

# #1437: Set c.hiddenRootNode.gnx:
g.app.nodeIndices.getNewIndex(v=c.hiddenRootNode)

Giving c.hiddenRootNode a valid (unique!) gnx should ensure that no other part of Leo can be affected by the dummy gnx.

Summary

The new code is straightforward. Its effects should be isolated to only the bridge itself.

Please let me know if I have misunderstood the problem, or its solution.

Edward

[Edward K. Ream]

On Friday, March 6, 2020 at 10:51:02 AM UTC-6, Edward K. Ream wrote:

> Rev 6269bab in the gnx branch follows Bob's third solution, more or less. Here are the details:

A simpler fix

`@test c.hiddenRootNode.fileIndex` failed with travisCI in the gnx branch. This failure suggested an even simpler solution:

>>> LeoFrame.createFirstTreeNode now always sets gnx to 'hidden-root-vnode-gnx'.

That's all! No change needed to leoBridge.py!

With this change, c.hiddenRootNode.gnx is always 'hidden-root-vnode-gnx'. This "dummy" gnx never changes, and it can never conflict with a "real" gnx.

Summary

The fix seems almost too good to be true. That said, I believe the fix is sound.

All unit tests now pass locally and with travisCI.

Bob, please test and report your experiences.

Edward

[vitalije]

I am not sure that this is a good idea. After createFirstTreeNode returns, the outline contains two different vnodes with the same gnx == 'hidden-root-vnode-gnx'. Some scripts might be broken by this. I am sure that I always assumed that 'hidden-root-vnode-gnx' is the gnx of c.hiddenRootNode. Now, with the proposed fix, there will be a vnode with this same gnx and yet it is not c.hiddenRootNode.

If giving the fixed gnx to this first created vnode is the solution for the bug, I suggesst that you use another constant  for gnx, not the one reserved for the c.hiddenRootNode.

Vitalije

[vitalije]

After reading the file, bridge.openLeoFile sets c.hiddenRootNode.gnx to a value that doesn't conflict with any gnx in the just-read file:

I am not convinced that the c.hiddenRootNode causes the gnx clash in the bridge. I suppose the first child of c.hiddenRootNode is the one vnode that can introduce gnx clashes in the bridge when opening two or more outlines within the same second. Perhaps in the bridge there is no need to create this first child at all.

As I understand creating this first vnode is only necessary to allow user to see something in the outline even if it is empty. But in the bridge user doesn't see anything and perhaps the user of the bridge should be responsible to create this first node at the appropriate moment. When using bridge to open existing Leo file, there would be no need to create this node before opening the file.

Vitalije