IDLE doesn't call os.fsync()

[gvanrossum]

BPO	36807
Nosy	@gvanrossum, @terryjreedy, @dhalbert, @serhiy-storchaka
PRs	bpo-36807: When saving a file in IDLE, call flush and fsync #13102 [3.7] bpo-36807: When saving a file in IDLE, call flush and fsync (GH-13102) #13280 bpo-36807: Fix typo in NEWS item about IDLE (os.flush() should be os.fsync()) #13284 [3.7] bpo-36807: When saving a file in IDLE, call flush and fsync (GH-13102) #13288 [2.7] bpo-36807: When saving a file in IDLE, call flush and fsync (GH… #13293

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/terryjreedy'
closed_at = <Date 2019-05-13.23:47:53.669>
created_at = <Date 2019-05-05.20:22:26.300>
labels = ['3.8', 'expert-IDLE', 'type-bug', '3.7']
title = "IDLE doesn't call os.fsync()"
updated_at = <Date 2019-05-13.23:52:32.100>
user = 'https://github.com/gvanrossum'

bugs.python.org fields:

activity = <Date 2019-05-13.23:52:32.100>
actor = 'gvanrossum'
assignee = 'terry.reedy'
closed = True
closed_date = <Date 2019-05-13.23:47:53.669>
closer = 'terry.reedy'
components = ['IDLE']
creation = <Date 2019-05-05.20:22:26.300>
creator = 'gvanrossum'
dependencies = []
files = []
hgrepos = []
issue_num = 36807
keywords = ['patch']
message_count = 19.0
messages = ['341474', '341480', '341486', '341487', '341490', '341491', '341494', '341680', '341850', '341851', '342274', '342275', '342313', '342353', '342360', '342366', '342397', '342400', '342401']
nosy_count = 4.0
nosy_names = ['gvanrossum', 'terry.reedy', 'dhalbert', 'serhiy.storchaka']
pr_nums = ['13102', '13280', '13284', '13288', '13293']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue36807'
versions = ['Python 2.7', 'Python 3.7', 'Python 3.8']

[gvanrossum]

This came up during today's final PyCon keynote -- IDLE was called out
as one of the two editors *not* to use when live-coding on Adafruit's
Circuit Playground Express (https://www.adafruit.com/product/3333).

I *think* that the problem referred to is that IDLE doesn't guarantee
that the bits are actually flushed to disk -- they may linger in the
OS filesystem cache.

[gvanrossum]

If/when you accept this we should also backport it as far as we can.

[terryjreedy]

This seems like a good idea. I am assuming that open('somename', ...) always result in a file object with .fileno defined.

Although I am mostly not touching 2.7, this should be easy to do. It will have to be done manually since iomenu was then IOBinding.

[terryjreedy]

OS and disk interaction is not something I know a lot about. I don't know how long different OSes take to write things out by themselves, and therefore how much real danger there is of loosing data. However, IDLE is used in places where power is less reliable than it is for me, and if not doing this makes IDLE look bad, and if it does not noticeably delay running a file (and I expect not), then it seems like a good idea.

Digging further, Kurt Kaiser added f.flush in 3/19/2006. On 2013-08-04, for bpo-18151, Serhiy submitted a patch for 3.3 with the comment "Here is a patch which replace the "open ... close" idiom to the "with open" idiom in IDLE." It replaced
f = open(filename, "wb")
f.write(chars)
f.flush()
f.close()
with
with open(filename, "wb") as f:
f.write(chars)

I have no idea why f.flush was deleted. An oversight? There is no discussion on the issue. I merged Serhiy's patch and backported to 2.7.

[terryjreedy]

The io doc says for IOBase flush()
Flush the write buffers of the stream if applicable. This does nothing for read-only and non-blocking streams.

and for BufferedWriter flush()
Force bytes held in the buffer into the raw stream. A BlockingIOError should be raised if the raw stream blocks.

On 3.x, open(filename, "wb"), used in writefile(), returns a BufferedWriter. So it seems than an exception is possible, which would crash IDLE without try-except.

Serhiy, please read the previous message(s). Do you remember if you intended to remove the f.flush in writefile(), which Guido proposes to restore?

[serhiy-storchaka]

f.flush() was removed because it is redundant if followed by f.close(). f.close() calls f.flush() internally. f.close() is now called implicitly in __exit__() when exit from the with statement.

Guido restores f.flush() because he needs to flush the buffer of the buffered stream before calling os.fsync(f.fileno()). And f.fileno() can be used only for not closed file. So this change looks reasonable to me.

I do not know what are the troubles with Adafruit's
Circuit Playground Express. I think that flushing the OS filesystem cache is the work of the OS itself, and lingering the data in the OS-wide cache should not affect userspace. But maybe there are specific circumstences, for example if IDLE and the consumer of Python source files are ran on different machines.

[gvanrossum]

That board implements a USB filesystem that you plug into a computer (via a cable). The control software on the board watches this USB filesystem, and when the "code.py" file changes it reloads that file and executes it with CircuitPython (Adafruit's fork of MicroPython).

So the recommended workflow for the user is: edit the code to program a LED blinking pattern (for example); save the file; watch the LEDs on the board blink in the programmed pattern. Repeat with other changes. Endless fun.

Where IDLE currently doesn't cooperate is that after a save operation, the OS kernel doesn't immediately flush the bytes to the USB filesystem, so the board doesn't see the changes to the file until the OS cache gets flushed at the kernel's whim. (The board uses very low level fs operations because it's an 8-bit microprocessor with very little memory and therefore has very primitive software.)

The os.fsync() call ought to fix it by forcing the kernel to flush the bytes to the USB filesystem right away. This also helps when the user saves the file and immediately pulls out the USB cable. The OS will issue a warning in an attempt to train users to "Eject" first, but beginners are prone to making this mistake repeatedly.

Hope this helps. You should really watch @nnja's keynote once it goes online, it was really cool. :-)

[dhalbert]

I'm one of the CircuitPython core devs. This issue is OS-dependent: Windows and Linux don't necessarily write data and metadata out to USB drives promptly. The problem is particularly acute for FAT12 filesystems on Windows, which are typically 16MB or smaller: Windows can take up to 90 seconds to flush, due to a driver bug or perhaps some ancient concerns about floppy drives. MacOS doesn't have this issue.

See https://superuser.com/questions/1197897/windows-delays-writing-fat-table-on-small-usb-drive-despite-quick-removal/1203781 for details. We have contacted Microsoft about this but getting it fixed is a long-term and uncertain process.

I'll test on Windows when I return from PyCon. Thank you for the prompt fix!

[terryjreedy]

I watched the talk by Nina Zakharenko at https://www.youtube.com/watch?v=35mXD40SvXM. Since I love being able to save, compile, and run a file with one keystroke, I appreciate people wanting to do the equivalent with CircuitExpress and Adafruit. In IDLE, print() output would appear in IDLE's Shell unless sys.stdout were replaced.

Dan's note suggests that flush/fsynch would be more generally useful than for the board. I am inclined to add try: except: around the calls, and perhaps display a message "File was not saved" + traceback.

Dan, slightly OT, but I am curious whether one can access USB ports (in a system-dependent manner) directly from python code via os.system and ctypes?

[dhalbert]

Dan, slightly OT, but I am curious whether one can access USB ports (in a system-dependent manner) directly from python code via os.system and ctypes?

Do you mean from CircuitPython? The USB impplementation provides HID keyboard, mouse, and gamepad devices, CDC (serial), MIDI, and MSC (CIRCUITPY). We don't provide ctypes (though MicroPython does), but there are native modules that provide access to these devices. We will are planning to add user-defined USB descriptors.

Also, we provide some enhancements in a CircuitPython-specific module to peek for input on CDC, so you can know when to call input(), without using the usual OS-dependent Python tricks for this.

Right now REPL input/output is mixed with user input, as you mention, but we're considering adding a second serial CDC port so you'll be able to have a second serial channel and avoid colliding with the REPL.

I'd be happy to discuss this kind of stuff with you further, by email or whatever other mechanism you have in mind, like some appropriate python mailing list or our own chat servers, etc.

[dhalbert]

Fix tested and works! Comment from PR duplicated here.

I tested this fix by editing the 3.7.3 IDLE code by hand, and editing this test program as code.py on a CIRCUITPY drive on Windows 10:

import time
i = 0
while True:
    print(i)
    i += 1
    print("""\
123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890
123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890
""")
    time.sleep(1)

I typically test write flushing by editing a short program like this, and duplicating the print lines so that the size of the program grows by >512 bytes. This forces a rewrite of the FAT12 information because the program has increased in size by one or more filesystem blocks. Then I shrink it back down again. If file flushing doesn't happen immediately, the program will often throw a syntax error (because it's missing some blocks), or CircuitPython will get an I/O error, which is what happened before I patched IDLE.

So this looks good!

[gvanrossum]

@terry please see my comment on the PR.

[terryjreedy]

New changeset 4f098b3 by Terry Jan Reedy (Guido van Rossum) in branch 'master':
bpo-36807: When saving a file in IDLE, call flush and fsync (bpo-13102)
4f098b3

[terryjreedy]

When 3.x is done, I will do 2.7.

[terryjreedy]

New changeset 68a11b6 by Terry Jan Reedy (Miss Islington (bot)) in branch '3.7':
[3.7] bpo-36807: When saving a file in IDLE, call flush and fsync (GH-13102) (bpo-13288)
68a11b6

[terryjreedy]

PR 13102 is original patch for master; it was merged, not closed.
PR 13284 fixed blurb for master.
PR 13280 backport for 3.7 was closed trying to restart bogus failure; can't do that for backports.
PR 13288 is second backport; only optional Azure Pipilines failed.
PR 13293 is manual 2.7 backport.

[terryjreedy]

New changeset 353f8d2 by Terry Jan Reedy in branch '2.7':
[2.7] bpo-36807: When saving a file in IDLE, call flush and fsync (GH-13102) (GH-13293)
353f8d2

[terryjreedy]

Do you mean from CircuitPython?

No, I was asking about regular Python on Windows. I was not clear to me whether any of the software that Nina ran (other than possibly Adafruit USB drivers) was from Adafruit. I am now guessing that is was all 3rd party stuff and that IDLE will now be an alternative for part of what she used.

I was in part wondering whether there were other simple (and allowable) changes to IDLE that might make it more useful for this type of application, but I will leave that sit for now.

I answered at least some of my questions about USB and the Adafruit hardware and the rest of what you wrote with my search bar.

[gvanrossum]

Thanks all!