X-Sharp / XSharpPublic

Public repository for the source code for the XSharp Compiler, Runtime, Project System and Tools.
Apache License 2.0
113 stars 38 forks source link

Error not returned when automatically opening a structural index #1358

Closed leon-ts closed 9 months ago

leon-ts commented 1 year ago

When opening a DBF file, if the RuntimeState.AutoOpen setting is set to True, then the structural index is also opened. If there is an error in the index file being opened, the RDD detects it and throws an exception, but the exception is not translated back to where the DBF file was opened. As a result, the VODBUserArea function returns the success of the operation, despite the fact that the index file is corrupted. In the future, this undetected problem generates strange (from the user's point of view) errors. For example, NullReferenceException.

The problem was found in a real application. But I made a small example based on it, which clearly demonstrates the problem.

The following example has two files: FILE1.DBF and its structure index FILE1.CDX. An error was intentionally (for example) introduced into the index file. There was a typo in the saved index expression STR(COL1,4): the first character 'S' was replaced with 'R'. The result was an invalid expression with an unknown RTR function: RTR(COL1,4).

The VODBUseArea function opens FILE1.DBF and FILE1.CDX without errors. But then, when executing the VODBAppend function, a NullReferenceException is thrown. I took the debug XSharp.Rdd.dll and determined that this is happening due to the fact that some of the data in the classes, such as CdxTag, etc., was not initialized.

Example project (the file is opened from the 'Debug\Files\' folder): XSharpBetaTest.zip

Environment: Compiler X# 2.17.0.3 IDE VS 2022

cpyrgas commented 11 months ago

I am not sure that this change is a good idea, as it makes the behavior incompatible to VO, in which DBUSeArea() returns true with this file. Maybe it would be better to keep returning TRUE (since the dbf itself was opened), but also show an error dialog or raise some other flag indicating the corrupted index?

leon-ts commented 11 months ago

Hi Chris, The problem is that further work with a file in which a corrupted index is open is essentially impossible. Therefore, why not immediately inform the user about this? As for compatibility with VO, in this particular case we need to understand whether this feature could somehow be used by developers (the index is corrupted, DBUseArea did not report an error and we continue to work with the file), and whether the new behavior will negatively affect existing algorithms .

However, a compromise solution may be to not add the corrupted index to the workarea (as if the VODBOrdListAdd function had failed). In version 2.18.0.4 and previous versions, the problem is that the corrupted index objects are present in the workarea and runtime works with it as if everything is fine. As a result, incomprehensible errors arise during operation. And if DBUseArea returns True, but then I call, for example, DBSetOrder, but it returns FALSE due to the lack of a mounted index (which was not mounted in DBUserArea because it was corrupted), then this will be a normal compromise.

P.S. It is better to call ErrorDialog only on the UI thread. Otherwise, the code that calls it from a separate NonUI thread will create problems (in X# we can use multi-threaded programming, including in relation to RDD).

cpyrgas commented 11 months ago

Robert, Yeah, I understand the implications, it's just that I'm pretty sure there will be existing code that depends on the VO behavior... Btw, in VO, issuing a SetOrder(1) after opening the file returns FALSE, so that's also some indication that something went wrong.

leon-ts commented 9 months ago

The issue described in this ticket has been resolved. The VODBUseArea function returns False and RuntimeState.LastRddError contains the correct error description. But given the point Chris made, I'll leave the ticket closure to his discretion.

cpyrgas commented 9 months ago

Thanks Leonid, let's close it then! Will open it again in case anyone complains.