Ticket 480, PipeFS hangs on open for read when open for write
Pages: 1 2
Jeffrey Lee (213) 6048 posts |
This is a bit of an interesting one – attempting to open a PipeFS file for reading will block if the file is already opened for writing. Tracking down the cause took a while, running various combinations of new & old modules on RISC OS 3.7, since that seemed to be immune to the bug. And it turns out that it’s not a bug, it’s a feature (but a feature that was being hidden by a bug, and only appeared when another feature was implemented elsewhere). What happens is that when you open a file, FileSwitch calls the filesystem’s Of course in this case FileSwitch doesn’t care about the size of the file, it’s only checking to see what object type it is (file/image file/directory/none). So the blocking could be avoided if FileSwitch was changed to use But if blocking is the intended behaviour, then why does it only block on RISC OS 5? And if you try using OS_File 5 to read the size of a pipe which is open for write, why does it claim the file doesn’t exist? The reason for this is twofold: 1. When the OS_UpCall 6 call (made from within The On newer OS versions with TaskWindow 0.65+, the better implementation of OS_UpCall 6 means that instead of returning immediately, Fixing the register corruption bug in PipeFS will make it so that everything works “correctly”, regardless of TaskWindow version – OS_File 5, *FileInfo, and OPENIN will all block until the file is closed for writing, and OS_File 5 & *FileInfo will now correctly report the file info instead of claiming it doesn’t exist. Of course it’s still not ideal that OPENIN blocks, but as mentioned above, that could probably be fixed by changing FileSwitch to use But then there’s OS_File 5, which will now block instead of claiming the file doesn’t exist. Is anyone aware of any software which could break because of this? (e.g. if the same thread that opened the file for writing tries to read the properties of the file it’ll deadlock itself) And similarly, would anything break if we removed the It’s also interesting that the PRMs suggest using OS_GBPB 10 to read the length of PipeFS files. Is that because the person writing the documentation is expecting OS_File 5 to block, or is it because they’d observed that OS_File 5 sometimes claims that files don’t exist? Note that the code which implements OS_GBPB 9-12 doesn’t contain any |
|||||||||||||||||||||
Sprow (202) 1150 posts |
You’re up at 2am, so that’s dedication!
That reason code was added by SKS according to the change log in HdrSrc, perhaps Stuart can add insight? The PRM mentions its use with NetFS to avoid having to call the file server, whether that’s a speed optimisation to avoid asking, or a special just for
I think if I did an OS_File 5 on a file that didn’t exist in PipeFS I’d expect that to return that the file didn’t exist, as it’s not in the catalogue. If the file is open for writing though, I would expect it to block until closed. I think that’s what you’re proposing. A quick search shows various of the Configure plugins, AcornSSL, perl, Env, (TaskWindow/PipeFS) all use PipeFS if you need a guinea pig. |
|||||||||||||||||||||
Stuart Swales (8827) 1326 posts |
Looks like I added Dunno about PipeFS, after my time. I have memories of people BITD mentioning it as something one customer wanted and was implemented so that particular use case worked. I would imagine that was to have to writer start before the reader. |
|||||||||||||||||||||
Jeffrey Lee (213) 6048 posts |
No, that’s more just a sign that it took longer than expected to translate thoughts to words.
That’s what will happen if I fix the For OS_File 5 of a file that’s open for writing, we have the following behaviours:
And for OPENIN:
|
|||||||||||||||||||||
Rick Murray (539) 13750 posts |
I have the following comment in my FS0 reader notes: For file length, you need to jump to the map information for the object and read the length from there... File size is ( ?8 + ( ?13 * 256) + (?14 * 64K) ) ?8 = LSB of object length (the rest) ?13 = LSB of contiguous blocks (256 byte blocks) ?14 = MSB of contiguous blocks (64K blocks) And for non-continuous blocks? So it looks like the FileStore has to do at least two reads to determine the size of a file (the directory, then the map entry prefixing the file), possibly more depending on whether or not files are always kept in contiguous sectors. I didn’t think the FileStore did auto-compaction, but a quick look at the ROM dump doesn’t give any indication of a command like *FSCompact… |
|||||||||||||||||||||
nemo (145) 2496 posts |
Jeffrey observed
But then claimed
No, this pattern has always been faulty and is nothing to do with open files. Files can easily change size on networked and hosted filing systems, but even a stand-alone RO may have tickers, callbacks, vector claimants or event handlers that log to a file or otherwise open-append-close in the background. This pattern is fixed by the SaferOSFile module which adds an optional buffer length to the OS_File,12/14/16/255 API, and I’d encourage people to always use the safe version of those APIs as it’s 100% backwards compatible. |
|||||||||||||||||||||
Dave Higton (1515) 3478 posts |
I thought the whole idea of a pipe filing system was to allow simultaneous reads and writes? Well, if not the whole idea, at least a valuable feature. If you don’t allow simultaneous reads and writes, then you might just as well write a file, close it, and read it; which you can do on any filing system. |
|||||||||||||||||||||
Steve Pampling (1551) 8125 posts |
Sounds like the way the OS ought to be |
|||||||||||||||||||||
Jeffrey Lee (213) 6048 posts |
Making FileSwitch use https://gitlab.riscosopen.org/RiscOS/Sources/FileSys/FileSwitch/-/merge_requests/14
Yes I was aware of that, I just neglected to mention it. |
|||||||||||||||||||||
Rick Murray (539) 13750 posts |
I think the window of opportunity is vanishingly small – it’s the duration between reading the file size, allocating the memory, and trying to load the file… but it’s possible. I’m pretty sure I once ran into an issue where a file was changed on the server but NetFS returned the (smaller) size that it had just recently read from the server, not the new size. Was this RISC OS 2, perhaps? The vanishingly small window of opportunity is rather larger when it’s a slow floppy-based server with multiple clients trying to do stuff at the same time. ;-)
Yeah, it’s a bit mad not to ask for and respect a buffer size. This is why I always load data with HeeBeeGeeBee. It’s not any safer in things changing in the background, but one gives it a count of how many bytes to read so there’s no risk of overrun. |
|||||||||||||||||||||
Simon Willcocks (1499) 506 posts |
Anything that can happen, will happen. Anythnig bad, anyway. |
|||||||||||||||||||||
Stuart Swales (8827) 1326 posts |
Just with sufficiently low frequency that you never track it down. I recall there being a bug in the Cambridge Ring that would eventually stiff systems after six months. So you just had to remember to reboot every couple of months… |
|||||||||||||||||||||
nemo (145) 2496 posts |
Rick remarked
Indeed, buffer overrun is a real risk, and SaferOSFile uses that method internally of course. And the reason that module exists is I did actually hit the problem on a hosted system.
‘Small’ is a relative term. Compared with your computer, your ability to add up is ‘small’. Given a Basic program in a TaskWindow doing OS_File,5, displaying the results, DIMing a buffer and OS_File,255ing the data, there’s plenty of opportunity for change even on a stand-alone RO. |
|||||||||||||||||||||
nemo (145) 2496 posts |
Stewart suggested
Which reminds me… the next Bad Time To Save is between 05:22:34 08-Nov-2023 and 08:17:17 08-Nov-2023, which is sufficiently early that it’s unlikely to affect anyone. [BTTS: There is a very small possibility that a file saved during such a period will lose its filetype. This is caused by a combination of coincidence and nostalgia.] |
|||||||||||||||||||||
Clive Semmens (2335) 3274 posts |
The idea that computers suffer from nostalgia is an interesting one, albeit clearly real. |
|||||||||||||||||||||
Steve Pampling (1551) 8125 posts |
Sometimes they do unwanted things just to get you wound up, ask any IT support bod. |
|||||||||||||||||||||
nemo (145) 2496 posts |
In this case a codified but not clearly documented veneration of the ancients. My version of Filer tries to be a lot more intelligent about unstamped but identifiable files for the sake of !65Host and !BeebIt, but results are mixed – the DTP protocol is unclear (surprise!) about the filetype at +&40 and what the recipient should do when it does not match that of the file itself (which is a perfectly normal function – e.g. shift-double-click in Filer). The lie in the message should cause the file to be loaded, but should not be used as the filetype thereafter. |
|||||||||||||||||||||
Rick Murray (539) 13750 posts |
;) Except those of us who run servers and periodic logging. Why is it’s bad time to save? We can’t be flipping on the high bit as it’s a period of about three hours. There’s nothing marked in my astronomical diary for that day. And it’s National Cappuccino Day in America. Hmm…? |
|||||||||||||||||||||
Rick Murray (539) 13750 posts |
There’s your problem right there. If the Filer sends a message with a filleule, it’s entirely reasonable for the receiver to treat that as correct. What the Filer should have done is have a flag bit to say “load this, don’t run it”, rather than blatant lying to fudge the desired behaviour. |
|||||||||||||||||||||
nemo (145) 2496 posts |
Coincidence and nostalgia: Timestamps and FileType are stored in the Load and Execution address metadata. Twelve bits must be set, the FileType is 12 bits, and the remaining 40 bits are the timestamp in cs. At unfortunate moments the least significant 32bits of the timestamp can match the combination of set bits, filetype and most significant byte of the timestamp, and hence Load=Exec. However, for historical reasons if the Load=Exec then the OS (by which I mean FileSwitch and Filer) regard the file as unstamped. Bad luck! Significantly, nothing tries to avoid that outcome by incrementing the cs – it just goes ahead and produces unusable metadata. Sigh. |
|||||||||||||||||||||
nemo (145) 2496 posts |
Rick mangled
Assuming you meant it’s reasonable for senders of DataLoad and DataOpen to lie about the filetype, you are correct, though you’d be hard pressed to find any documentation that states that.
DataOpen means ‘run’ in this sense, and DataLoad means ‘load’. Another pertinent point that is not generally considered is there is significant semantic difference between a Broadcast DataOpen (say) and one that has been sent direct to the app in question. There is an argument that in some cases an app might ignore the former but must always honour the latter. In reality there’s a lot to be desired in the DTP, which is why my Filer uses an additional DTP message – DataCheck – and many of my apps have individually-configurable filetype affiliation: |
|||||||||||||||||||||
Stuart Swales (8827) 1326 posts |
Wonders what Panos thinks of ‘stamped’ files with the top 24 bits of load/exec set, but Load=Exec. |
|||||||||||||||||||||
nemo (145) 2496 posts |
Didn’t Panos rely on file extensions, effectively? I forget the details. (i.e. that’s where the blasted “c.foobar” comes from, due to the large size of ADFS directories) |
|||||||||||||||||||||
Stuart Swales (8827) 1326 posts |
I wanted timestamped files in Arthur (and on the Master – see Edit), so just nicked the 5-byte cs time in load/exec idea from Panos, keeping the top 12 (20) bits set for vague compatibility – strictly Panos required the top 24 bits set. I think Panos would still recognise files set with I/O processor addresses and load them in there. |
|||||||||||||||||||||
nemo (145) 2496 posts |
Loved Edit. Used Twin throughout Arthur because of Edit.
So I/O Fxxx load addresses would be vulnerable if there were any RAM above E000… which fortunately there wasn’t. Random bit of fun M128 trivia: Not only could the 6845 use the ‘shadow RAM’ from 3000-7FFF, it could also access the ‘MOS RAM’ at 8000-8FFF when paged in (addresses 2000-2FFF mapped there). A 640×304 ‘MODE0’ might have been possible. |
Pages: 1 2