Issue #22: HostFiles.NewWriter violating the specification

cfbsoftware
Posts: 204
Joined: Wed Sep 18, 2013 10:06 pm
Contact:

Re: B1: HostFiles.NewWriter is violating the specification

Post by cfbsoftware »

Ivan Denisov wrote:The other solution is to make new procedure in Files.
You must have read my mind ;)

This discussion demonstrates the fact that before deciding on any solutions we need to clearly understand the problem that is supposedly being solved and then there needs to be a rigorous process to make sure it is not creating more problems than it is solving. The process goes something like this:

1. Identify a problem.
2. Report the problem.
3. Supply a test program that illustrates the problem.
4. Document some possible alternative solutions. This would typically be two or three - not just the first one thought of.
5. Analyse the pros and cons of each solution:
e.g. efficiency, complexity, resource usage, possible side-effects, does it fix the problem entirely or is it a workaround, how likely is it to break existing code etc. etc.
6. Decide on a solution.
7. Implement it.
8. Run the test case and run regression tests on other potentially affected areas.
9. Update the documentation.
10. Write release notes.
11. Release it.

For the issue currently being discussed, adding a new procedure minimises the risk of breaking existing code and the extent of regression testing required. I would not be confident in saying that is the best solution until steps 1 to 5 have been completed.
User avatar
DGDanforth
Posts: 1061
Joined: Tue Sep 17, 2013 1:16 am
Location: Palo Alto, California, USA
Contact:

Re: B1: HostFiles.NewWriter is violating the specification

Post by DGDanforth »

Ivan Denisov wrote: This experience with HostFiles shows that each bugfix should be discussed. Doug, for managing this "more than 50 changes" you should add the topic for each change for discussion on the forum happen, after the discussion we will put it on the issues tracker if a format of (issue + solution) and later commit this changes in downloadable repository.
I am not happy with the way the Center is moving. My vision of how it should operate is the following.

(1) Center members should be split into two people teams. Each team has assigned a set of issues (a few issues). The team privately emails back and forth between the two members their resolution to an issue until they are happy with it. They then place their solution into a "possible solution" set.
(2) The set of possible solutions are then used to build a new (tentative) version of the framework.
(3) The tentative version is then run (in an automated manner) against a set to test applications. Any failures of the tests are localized and isolated and returned to the team responsible for the failures. The teams then resubmit their new solution for version and this is iterated until no failures occur.
(4) The version is then made official (once all known issues have been included in the version) and released to the public.

Voting on each issue is not workable, much too slow, and forces everyone to know everything about all issues. That is way too much overload. Even though I am retired and have free time, it takes me a long time to master and dissect even small issues.

(For instance, the question about file writers does not touch me for I have never needed to reuse a writer. To really delve into that issue I would need to know under what circumstances reuse of a writer is needed and whether those circumstances are significant enough to require writer reuse)

Let's speed the whole process up and get people working on issues.

-Doug
User avatar
Josef Templ
Posts: 2047
Joined: Tue Sep 17, 2013 6:50 am

Re: B1: HostFiles.NewWriter is violating the specification

Post by Josef Templ »

Ivan Denisov wrote:The argument was, that there is no way to now is there file shared or not with a help of framework. That is why this TRAP becomes unavoidable.

There are one rule in Files documentation.
Opening a file in shared mode is the rule in BlackBox; opening a file in exclusive mode is an infrequent exception.
So, this problem should not appear if you led this rule.

However, how to prevent TRAP in infrequent exceptions? That means that programmer should provide this check with it's own module variables... and make strong encapsulation of files variables.

One File can be open with dir.Old other made with dir.New. You can not distinguish can you make file.NewWriter or not by having only the Files.File variable.
By checking the return value NIL you cannot see if the file is writable or not.
Why? Because if the system is running out of memory it also returns NIL when you call NEW.
So, returning NIL does not solve the problem, which is very exotic, by the way.
I have never experienced it and I can hardly imagine that it is relevant in praxis.
Typically, you know what you are doing with your files anyway.

If it is really important, the correct solution would be to
provide a getter function that returns the file's shared state.
Since the returned information corresponds with the 'shared' parameter of 'Old',
I would name it Shared(), not Writeable().
This would make the correspondence of the parameter and the getter explicit.

The same problem occurs with closed files.
As far as I know, there is no way to find out if a file has been closed or not.
Again, this has not been a problem for many years, but it is
the same kind of problem as with shared files.

Now we can introduce another getter named Closed(), but if nobody calls
it, it is really the question if we need it at all and if we need it now.
If, however, we introduce one, we should introduce the other as well.
The argument in favor of introducing them now is that now we make
incompatible binaries anyway, requiring a recompile of all user modules.
So this change would not cause any additional burden at the side of the users.

- Josef
Ivan Denisov
Posts: 1700
Joined: Tue Sep 17, 2013 12:21 am
Location: Russia

Re: B1: HostFiles.NewWriter is violating the specification

Post by Ivan Denisov »

So, we come to the solution of adding two procedures to Files?

Close() and Shared()

This fix do not need tests and so on. Do you think, that we can make the vote for including this into next release?
User avatar
ReneK
Posts: 214
Joined: Tue Sep 17, 2013 9:16 am
Location: Vienna, Austria, Europe

Re: B1: HostFiles.NewWriter is violating the specification

Post by ReneK »

Or we can speed up the decision on teams/distributing the work (there's a propasal for this) and bug tracking and have a clean way of doing things.
User avatar
Josef Templ
Posts: 2047
Joined: Tue Sep 17, 2013 6:50 am

Re: B1: HostFiles.NewWriter is violating the specification

Post by Josef Templ »

The important thing is to align the documentation with the implementation.
Adding Shared() and Closed() is an add-on that is rarely used if it is used at all.

@Helmut: what do you think?

- Josef
User avatar
ReneK
Posts: 214
Joined: Tue Sep 17, 2013 9:16 am
Location: Vienna, Austria, Europe

Re: B1: HostFiles.NewWriter is violating the specification

Post by ReneK »

DGDanforth wrote:I am not happy with the way the Center is moving. My vision of how it should operate is the following.

..

Voting on each issue is not workable, much too slow, and forces everyone to know everything about all issues. That is way too much overload. Even though I am retired and have free time, it takes me a long time to master and dissect even small issues.

..

Let's speed the whole process up and get people working on issues.

-Doug
I agree with your assessment, Doug. While there can be discussion on any bug fix, I think that discussion by all members will simply not happen. I think your post should be a thread of its own, so that we can discuss it properly, as it seems to not have been seen by those who took part in the discussion so far (at least there was no reaction to it).
Ivan Denisov
Posts: 1700
Joined: Tue Sep 17, 2013 12:21 am
Location: Russia

Re: B1: HostFiles.NewWriter is violating the specification

Post by Ivan Denisov »

I am suggesting to vote for fixing the documentation and adding Close() and Shared() function to Files.
User avatar
DGDanforth
Posts: 1061
Joined: Tue Sep 17, 2013 1:16 am
Location: Palo Alto, California, USA
Contact:

Re: B1: HostFiles.NewWriter is violating the specification

Post by DGDanforth »

Ivan Denisov wrote:I am suggesting to vote for fixing the documentation and adding Close() and Shared() function to Files.
Documentation for Files
"If an old writer is passed as parameter, it is the application's responsibility to guarantee that it is not in use anymore. Passing an unused old writer is recommended because it avoids unnecessary allocations.
Read-only files allow no writers at all. In such cases, NewWriter returns NIL."

Code in HostFiles
"ASSERT(f.state # closed, 20); ASSERT(f.state # shared, 21);"

Are you saying the two forms are inconsistent? HostFiles will TRAP if f.sate=closed whereas the documentation says the file should be closed (not in use anymore). Is that the point?
Ivan Denisov
Posts: 1700
Joined: Tue Sep 17, 2013 12:21 am
Location: Russia

Re: B1: HostFiles.NewWriter is violating the specification

Post by Ivan Denisov »

No. We need to remove:
"In such cases, NewWriter returns NIL."

Because it does not returns NIL, but TRAP.
Post Reply