classification
Title: test_os.test_memfd_create() fails on AMD64 FreeBSD Shared 3.x
Type: Stage:
Components: Tests Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: christian.heimes, kevans, koobs, lukasz.langa, vstinner
Priority: deferred blocker Keywords:

Created on 2020-06-17 22:52 by vstinner, last changed 2020-07-28 05:00 by koobs.

Messages (15)
msg371780 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-06-17 22:52
https://buildbot.python.org/all/#/builders/152/builds/1024
...
test_makedir (test.test_os.MakedirTests) ... ok
test_mode (test.test_os.MakedirTests) ... ok
Timeout (0:25:00)!
Thread 0x0000000800b54000 (most recent call first):
  File "/usr/home/buildbot/python/3.x.koobs-freebsd-564d/build/Lib/test/test_os.py", line 3520 in test_memfd_create
  ...
msg371792 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2020-06-18 06:00
Commit bb6ec14479f18c32e71e43f2785f177aa17aabbd fixed a problem with tests, so the test is now executed. Does FreeBSD have an implementation of memfd_create that behaves slightly differently than memfd_create on Linux?
msg371799 - (view) Author: Kubilay Kocak (koobs) (Python triager) Date: 2020-06-18 09:40
memfd_create  came in via:

https://github.com/freebsd/freebsd/commit/575e351fdd996f72921b87e71c2c26466e887ed2

via review: https://reviews.freebsd.org/D21393

via kevans (cc'd)
msg371810 - (view) Author: Kyle Evans (kevans) Date: 2020-06-18 12:58
Interesting; I don't quite have time to look at the moment, but what is the test doing that it's ultimately timing out?

Our memfd_create is assumed to be compatible, with the exception that we don't yet support HUGETLB (but there are patches in progress for the vm that might make it feasible); in my experience most users of memfd_create weren't really using hugetlb, though.
msg371811 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2020-06-18 13:11
The traceback points to line 3520, which calls f.tell() on a file object that wraps a memfd.
msg371814 - (view) Author: Kyle Evans (kevans) Date: 2020-06-18 14:06
I think it's too early to look at this, I'll circle back another time. :-) I got a minute and extracted the relevant test, but I guess there must have been some fundamental transcription error:

import os

fd = os.memfd_create("Hi", os.MFD_CLOEXEC)
if fd == -1:
    print("boo")
    os.exit(1)

if os.get_inheritable(fd):
    print("inheritable, boo")
    os.exit(1)

with open(fd, "wb", closefd=False) as f:
    f.write(b'memfd_create')
    if f.tell() != 12:
        print("Hork")
        os.exit(1)
    print("What?")

print("Done")

When I run this with python3.9, I get:

...
 3038 100554: shm_open2(SHM_ANON,O_RDWR|O_CLOEXEC,00,0,"memfd:Hi") = 3 (0x3)
 3038 100554: fcntl(3,F_GETFD,)          = 1 (0x1)
 3038 100554: fstat(3,{ mode=---------- ,inode=10,size=0,blksize=4096 }) = 0 (0x0)
 3038 100554: ioctl(3,TIOCGETA,0x7fffffffe2a0)   ERR#25 'Inappropriate ioctl for device'
 3038 100554: lseek(3,0x0,SEEK_CUR)      = 0 (0x0)
 3038 100554: lseek(3,0x0,SEEK_CUR)      = 0 (0x0)
 3038 100554: write(3,"memfd_create",12)     = 0 (0x0)
 3038 100554: write(3,"memfd_create",12)     = 0 (0x0)
 3038 100554: write(3,"memfd_create",12)     = 0 (0x0)
 3038 100554: write(3,"memfd_create",12)     = 0 (0x0)
 3038 100554: write(3,"memfd_create",12)     = 0 (0x0)
(ad infinitum)

So in my local repro, Python is looping forever on successful write() for reasons I'm not immediately sure of.
msg371815 - (view) Author: Kyle Evans (kevans) Date: 2020-06-18 14:10
(Transcription error beyond the bogus os.exit() :-))
msg371818 - (view) Author: Kyle Evans (kevans) Date: 2020-06-18 14:49
Ah, now that I'm more awake, I see the problem- the write is unsuccessful because I haven't yet implemented grow-on-write. None of the consumers that I had found relied on it, instead choosing to ftruncate() it to the size they need before operating on it.

I think the best course of action might be to disable the test on FreeBSD for now (unless you're ok with tossing in a truncate to expand the file), because it might take me a little bit to get around to this one.
msg372337 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-06-25 10:13
> So in my local repro, Python is looping forever on successful write() for reasons I'm not immediately sure of.

io.BufferedWriter.write() (well, especially its flush() method) calls write() until all data is written.

Extract of _bufferedwriter_flush_unlocked() code, Modules/_io/buffered.c:

    while (self->write_pos < self->write_end) {
        n = _bufferedwriter_raw_write(self,
            self->buffer + self->write_pos,
            Py_SAFE_DOWNCAST(self->write_end - self->write_pos,
                             Py_off_t, Py_ssize_t));
        if (n == -1) {
            goto error;
        }
        else if (n == -2) {
            _set_BlockingIOError("write could not complete without blocking",
                                 0);
            goto error;
        }
        self->write_pos += n;
        self->raw_pos = self->write_pos;
        written += Py_SAFE_DOWNCAST(n, Py_off_t, Py_ssize_t);
        /* Partial writes can return successfully when interrupted by a
           signal (see write(2)).  We must run signal handlers before
           blocking another time, possibly indefinitely. */
        if (PyErr_CheckSignals() < 0)
            goto error;
    }

You are correct: if write() returns 0, write() is called again. If write() always returns 0, the loop never stops...

Maybe a BlockingIOError must be raised if write(buffer) returns 0 (and buffer is not empty).
msg372678 - (view) Author: Ɓukasz Langa (lukasz.langa) * (Python committer) Date: 2020-06-30 11:05
As a regression this would have been a release blocker, this also fails on 3.9 and 3.8. However, given that this has only been surfaced by Christian's fix to the testing machinery in GH-20942, I'll mark this as deferred blocker instead for visibility.
msg372682 - (view) Author: Kyle Evans (kevans) Date: 2020-06-30 11:43
Ah, sorry, I meant to update this- I submitted our fix for review a day or two ago, got approval for commit and will poke koobs to rebuild the FreeBSD -CURRENT buildbot with it after that.
msg373597 - (view) Author: Kubilay Kocak (koobs) (Python triager) Date: 2020-07-13 12:17
I've updated the FreeBSD CURRENT buildbot past base/r363065 [1] which implements SHM_GROW_ON_WRITE:

https://svnweb.freebsd.org/changeset/base/363065
https://reviews.freebsd.org/D25502

Also worth noting that I don't believe stable/12 (FreeBSD 12.x) will be getting this syscall (correct me if im wrong Kyle), so these tests may still fail on the FreeBSD 12.x worker, and the tests should be updated to account for that in some manner.
msg373598 - (view) Author: Kyle Evans (kevans) Date: 2020-07-13 12:28
I can confirm that neither 12 nor 11 will be getting memfd_create; file sealing is a little more complicated to MFC, and I don't want to provide memfd_create in a place where it can't be paired with file sealing in case applications assume they come hand-in-hand and want to use sealing for freezable shm type stuff.
msg374457 - (view) Author: Kyle Evans (kevans) Date: 2020-07-28 03:26
Hey koobs,

Can you confirm that this is fine now after I implemented SHM_GROW_ON_WRITE?
msg374463 - (view) Author: Kubilay Kocak (koobs) (Python triager) Date: 2020-07-28 05:00
@Kyle Yes, msg373597 freebsd CURRENT buildbot passes, but freebsd 12/stable does not (cannot, since stable/12 wont get the syscall)

That means the Python memfd functions and tests need to account for this
History
Date User Action Args
2020-07-28 05:00:07koobssetmessages: + msg374463
2020-07-28 03:26:36kevanssetmessages: + msg374457
2020-07-13 12:28:54kevanssetmessages: + msg373598
2020-07-13 12:17:22koobssetmessages: + msg373597
2020-06-30 11:43:23kevanssetmessages: + msg372682
2020-06-30 11:05:07lukasz.langasetpriority: normal -> deferred blocker
versions: + Python 3.8, Python 3.9
nosy: + lukasz.langa

messages: + msg372678
2020-06-25 10:13:15vstinnersetmessages: + msg372337
2020-06-18 14:49:38kevanssetmessages: + msg371818
2020-06-18 14:10:28kevanssetmessages: + msg371815
2020-06-18 14:06:50kevanssetmessages: + msg371814
2020-06-18 13:11:32christian.heimessetmessages: + msg371811
2020-06-18 12:58:54kevanssetmessages: + msg371810
2020-06-18 09:40:05koobssetnosy: + kevans
messages: + msg371799
2020-06-18 06:00:12christian.heimessetnosy: + christian.heimes
messages: + msg371792
2020-06-17 22:52:21vstinnersetnosy: + koobs
2020-06-17 22:52:12vstinnercreate