ENB: Fixing #1437 properly

[Edward K. Ream]

This Engineering Notebook post discusses possible fixes to #1437.

Background

The hidden vnode is c.hiddenRootNode. The root vnode is c.rootPosition().v.

My botched fix confused these two vnodes. I thank several people for checking my work.

When creating any outline, Leo's startup code sets the hidden vnode, c.hiddenRootNode. This node itself never changes, but it's contents (v.children array) changes when top-level outline nodes are inserted or deleted.

Leo's startup code give the hidden vnode a unique, invariant, gnx, namely 'hidden-root-vnode-gnx'. This gnx can never conflict with any other vnode in the outline.

LeoFrame.createFirstTreeNode creates the root vnode. Leo will replace this root vnode by a new vnode if the outline corresponds to an external file.

The problem

For all outlines, Leo calls LeoFrame.createFirstTreeNode to create the root vnode. The root vnode is given a new gnx. Normally, this new gnx can't conflict with the gnx in any about-to-be-read .leo file. However, a conflict can happen when Leo's bridge reads several .leo files in less than a second.

The solution

I considered eliminating createFirstTreeNode entirely. However, this would cause ripple effects throughout Leo's startup code.The new code might well be more complex and dangerous than the existing code. I'll omit the gory details. Suffice it to say that I won't do this.

Instead, I plan to solve the problem entirely in leoBridge.py. Vitalije suggests a straightforward hack:

1. Before reading the .leo file, bridge.openLeoFile (via its helper, bridge.createFrame) will assign a dummy gnx to the root vnode. This dummy gnx will be different from c.hiddenRootNode.gnx.

2. After reading the file, bridge.openLeoFile will give the root vnode a new, unique, gnx only if it still contains the dummy gnx. This can happen if the given file name does not exist.

Summary

This is a tricky topic.This post omits discussion of various preliminary researches.

The proposed solution should work, but only because the bridge opens all outlines using bridge.openLeoFile.

I'll delay Leo 6.2 b1 for at least a week after merging the gnx branch into devel.

All questions, comments and suggestions are welcome.

Edward

P.S. At present, fc.getLeoFile calls checkOutline, which then calls c.checkGnxs. This ensures that all outline nodes have unique gnx's. It's not clear to me whether these checks will be needed after fixing #1437, but I have no plans to remove them.

EKR

[Edward K. Ream]

On Sunday, March 8, 2020 at 7:37:04 AM UTC-5, Edward K. Ream wrote:

This Engineering Notebook post discusses possible fixes to #1437.

Rev 4956d79 in the gnx branch contains my second attempted fix. Please test it and report your results.

And yes, I meant to say 6.2 b1, not 5.2 b1.

Edward

[Thomas Passin]

On Sunday, March 8, 2020 at 8:37:04 AM UTC-4, Edward K. Ream wrote:

This Engineering Notebook post discusses possible fixes to #1437.

The problem

For all outlines, Leo calls LeoFrame.createFirstTreeNode to create the root vnode. The root vnode is given a new gnx. Normally, this new gnx can't conflict with the gnx in any about-to-be-read .leo file. However, a conflict can happen when Leo's bridge reads several .leo files in less than a second.

Would it be possible to add a few more digits of time resolution to the gnx's?  The exact length or resolution isn't used anywhere, is it?  More resolution would seem an easy way to avoid gnx duplication. (It seems so simple that there must be a good reason why it wouldn't work).

[vitalije]

If we are speaking of "fixing 1437" properly, I have to say that the more I think about this bug, the more I am sure that the real problem is not inside createFirstTreeNode. This method is doing a quite normal, natural thing.

It should not be an error to have two nodes with the same gnx in two separate outlines. The bug 1437 is about exactly this. Commander created using leoBridge, has an outline with some freshly created gnx, then it replaces the whole outline with the new one read from the file. If that outline (read from the file) contains the node with the exactly same gnx, it fails to read it correctly.

I think there is no sense in keeping the content of c.fileCommands.gnxDict when loading completely new outline. If it is cleared inside the openLeoFile, before calling getLeoFile then it won't cause a gnx clash, and the outline will be read correctly.

def openLeoFile(self, theFile, fileName, readAtFileNodesFlag=True, silent=False):
    """Open a Leo file."""
    c, frame = self.c, self.c.frame
    # Set c.openDirectory
    theDir = g.os_path_dirname(fileName)
    if theDir:
        c.openDirectory = c.frame.openDirectory = theDir
    # Get the file.
    self.gnxDict = {} # clear gnxDict before replacing the whole outline
    ok, ratio = self.getLeoFile(
        theFile, fileName,
        readAtFileNodesFlag=readAtFileNodesFlag,
        silent=silent,
    )
    if ok:
        frame.resizePanesToRatio(ratio, frame.secondary_ratio)
    return ok

Using the following script to check:

import sys
sys.path.insert(0, '/home/vitalije/programi/leo/bug1437')
import leo.core.leoBridge as leoBridge
import leo.core.leoNodes as leoNodes
import pytest

def bridge():
    return leoBridge.controller(gui='nullGui',
        loadPlugins=False, readSettings=False,
        silent=False, verbose=False, useCaches=False)

def test_1437(fileName):
    # Create, save and close outline with a single node
    br = bridge()
    g = br.g
    c = br.openLeoFile(fileName)
    p = c.rootPosition()
    p1 = p.insertAfter()
    p1.h = "1"
    p.doDelete(p1)
    assert c.rootPosition().h == "1"
    c.save()
    c.close()
    g.app.nodeIndices = leoNodes.NodeIndices(g.app.leoID)
    # Reset the global state so a new bridge will come
    # with fresh nodeIndices. This simulates
    # SegundoBob's launching bridge consecutively in
    # multiple processes
    leoBridge.gBridgeController = None

    # Read the file and verify the headline is "1"
    c = bridge().openLeoFile(fileName)
    assert c.rootPosition().h == "1"
if __name__ == '__main__':
    try:
        for i in range(100):
            test_1437('/tmp/tree1.leo')
    except KeyboardInterrupt:
        sys.exit(0)

Without the line self.gnxDict = {}, assertion error happens almost always during the loop. With that line it doesn't. So, I guess it solves the 1437.

All unit tests pass.

Vitalije

[Edward K. Ream]

On Sun, Mar 8, 2020 at 10:27 AM Thomas Passin <tbp1...@gmail.com> wrote:

Would it be possible to add a few more digits of time resolution to the gnx's? 

No. As I read the docs for python's time module, the maximum guaranteed resolution is 1 second.

In any case, I don't want Leo to create huge gnx's, just a weird special case.

Edward

[Edward K. Ream]

On Sun, Mar 8, 2020 at 12:05 PM vitalije <vita...@gmail.com> wrote:

If we are speaking of "fixing 1437" properly, I have to say that the more I think about this bug, the more I am sure that the real problem is not inside createFirstTreeNode. This method is doing a quite normal, natural thing.

I agree. Perhaps I wasn't clear in the posting.

It should not be an error to have two nodes with the same gnx in two separate outlines.

It isn't an error. Paste retaining clones is intended to work between outlines. This command retains gnxs.

The bug 1437 is about exactly this.

Hmm. Maybe not exactly this, but close.

Commander created using leoBridge, has an outline with some freshly created gnx, then it replaces the whole outline with the new one read from the file. If that outline (read from the file) contains the node with the exactly same gnx, it fails to read it correctly.

That's what Bob asserts, and I believe him.

I think there is no sense in keeping the content of c.fileCommands.gnxDict when loading completely new outline.

Yes, that might work. However, I believe the code I pushed also works. Let me know if it doesn't.

Edward

[vitalije]

Yes, that might work. However, I believe the code I pushed also works. Let me know if it doesn't.

Edward

Using the test script from my previous message, your fix in gnx branch fails.

Vitalije

[Edward K. Ream]

On Sun, Mar 8, 2020 at 1:30 PM vitalije <vita...@gmail.com> wrote:

> Using the test script from my previous message, your fix in gnx branch fails.

Ok then. I'll use your solution.

Edward

[Edward K. Ream]

On Sunday, March 8, 2020 at 1:36:15 PM UTC-5, Edward K. Ream wrote:

Ok then. I'll use your solution.

Actually, I would prefer that you make the fix. That way there won't be any confusion about your intent.

Edward

[vitalije]

Actually, I would prefer that you make the fix. That way there won't be any confusion about your intent.

Edward

Done at  a57a57996 in the devel branch.

Vitalije

[Edward K. Ream]

On Sunday, March 8, 2020 at 2:10:20 PM UTC-5, vitalije wrote:

> Done at  a57a57996 in the devel branch.

Thanks for this! I've merged it into the xml and refs branches.

Edward