Fix: XNCI writes corrupting node data (alternative)

[zack-vii]

Proposal for a substitute of PR #2498

[WhoBrokeTheBuild]

I boiled down your changes to TreeSegmentTest in order to have a use case for testing:

import MDSplus
import numpy as np

tree = MDSplus.Tree('test', -1, 'NEW')
tree.addNode('t', 'NUMERIC')

tree.T.makeSegment(0, 2, np.array([0, 1, 2]), np.array([1, 2, 3]))
tree.T.record

tree.T.record = 7357
tree.T.record

tree.T.setExtendedAttribute("BEFORE", 7357)
tree.T.record # exception

I verified that this clobbers the data on the current alpha, and works as expected on your branch :tada:

[zack-vii]

sure, well for what I gathered the attribute's facilities are meant to store (among others) either a standard record or a segment header. I am not exactly sure why the header and standard record are not stored in the same memory although they are exclusive. The nci flags/flags2 store the information about the context as well. Nonetheless, sunce they re exclusive, when writing a segment, the standard record should be removed. In the same way writing a standard record into a segmented node removes the segmented header ref in TreePutRecord. Maybe when refactoring the code to reduce code duplication the begin_attributes method inherited the removal. however, when writing an xnci the standard record should of course be preserved. Changing the deleted length value to -1 is just to be consistant with the init value of attributes which is defined by memset of -1.

[mwinkel-dev]

Hi @zack-vii -- Thanks for the explanation.

I have repeated the experiments that Stephen ran. And obtained the same results.

I am now studying the changes (mostly to learn about XNCI). If the review doesn't prompt any additional questions, I will add my approval to this PR later today.

[zack-vii]

I happy to hear that. In case it is all good, feel free to merge the pr.

[zack-vii]

I verified that this clobbers the data on the current alpha, and works as expected on your branch 🎉

for testing it, you dont need to write a segment first. the test extension just tests also if segmented nodes stay intact. as well as the other segment tests regarding multithreading. using more than one thread in the test fails on ntfs filesystems because ofd locks are not supported. i think there was a related issue. I played with a solution to an alternate locking mechanism in case ofd lock are not supported.