Lwt: getcwd in lwt

[kennetpostigo]

I'm using lwt in a project and I have no previous experience with lwt, but is there a function in lwt for grabbing the current working directory? If not then is it safe to use Sys.getcwd with lwt while remaining async?

Aside from that command, is it safe to assume that any command in Unix, Sys, etc. that lwt did not wrap is safe to use in lwt while remaining async?

[aantron]

is there a function in lwt for grabbing the current working directory

I don't believe there is such a function in Lwt. I looked in the source code of various systems' standard libraries, and it seems that this is an oversight of Lwt. Unix.getcwd calls getcwd(3), and:

• On Linux, it appears that the default implementation basically just makes some kind of syscall (not sure if the macro is really a syscall, didn't look), and the docs suggest that it reads from /proc/self, but the actual implementation of that may or may not be capable of blocking. However, the point seems moot, because of the next points.
• On macOS, it appears that the implementation is always potentially-blocking, as it calls things like open(2), opendir(2), etc.
• There is a fallback POSIX (calling only other POSIX functions) implementation in Linux glibc, which also performs a whole bunch of operations that might block.

So, it seems that we should add an Lwt_unix.getcwd function.

is it safe to use Sys.getcwd with lwt while remaining async?

so the short answer is no. But, in practice, for now, you have three options:

1. If you are on Linux and/or can guarantee that your working directory is not in NFS or something like that, you can probably expect Unix.getcwd not to block. Also, you can, of course, just call a blocking operation in the middle of a bunch of Lwt code. It won't break the abstract correctness of anything, it just might slow things down considerably/indefinitely on occasion :/

2. Use Lwt_preemptive to detach a thread for running getcwd:

   let lwt_getcwd = Lwt_preemptive.detach Unix.getcwd
   
   (* or, fully eta-expanded, to be explicit: *)
   let lwt_getcwd () =
     Lwt_preemptive.detach (fun () -> Unix.getcwd ()) ()
   

3. Use uwt's Uwt.Unix.getcwd, which uses libuv's thread pool for running getcwd.

Aside from that command, is it safe to assume that any command in Unix, Sys, etc. that lwt did not wrap is safe to use in lwt while remaining async?

That is often true, but evidently not always. The best strategy for now is probably to read man pages. In this case, I unfortunately had to resort to reading source code (and I hope I found the right code...). What the Lwt project should do, when focus reaches improving the Unix binding, is comb through all of Unix, make sure everything that can block has an Lwt binding, and then document that Unix can definitely be safely used for the rest.

[aantron]

(and the above applies to Sys.getcwd as well, because it also calls getcwd(3))

[agarwal]

FWIW, Async's version of getcwd is non-blocking.

[kennetpostigo]

@aantron thank you for the info, is there some time table for how long it may take to get this into lwt? Also I think I may run into this often throughout my project should I open an issue for each of those occasions or should I contact you through some channel for these occurrences?

[mfp]

On Thu, Apr 20, 2017 at 09:58:43AM -0700, Kennet Postigo wrote:

@aantron thank you for the info, is there some time table for how long it
may take to get this into lwt? Also I think I may run into this often
throughout my project should I open an issue for each of those occasions or
should I contact you through some channel for these occasions?

aantron is carrying most of the burden of maintenance but this doesn't mean
you should funnel all the work directly into his inbox :) A separate issue for
each blocking syscall missing in Lwt is fine (we can always coalesce them if
needed).

Both Lwt 3.0.0 and 2.7.1 were released a few days ago, so there are no plans
for an immediate release. For the time being I'd suggest you just call
getcwd and other blocking functions (if any) in a Lwt_preemptive.detach block.
It's not really that different from the built-in job functionality in Lwt_unix
(which also runs blocking calls in separate threads) or what Async apparently
does FWIW:

https://github.com/janestreet/async_unix/blob/master/src/unix_syscalls.ml#L338

--
Mauricio Fernández

[kennetpostigo]

@mfp I totally understand. I probably should have worded it differently. I just wanted to make sure opening issues for these occurrences are the best course of action. You may want 1 issue that consolidates these missing from lwt. Thanks for the tips @aantron @mfp

[mfp]

On Thu, Apr 20, 2017 at 10:45:56AM -0700, Kennet Postigo wrote:

@mfp I totally understand. I probably should have worded it differently. I just wanted to make sure opening issues for these occurrences are the best course of action. Thanks for the tips @aantron @mfp

I apologize if I came out as overly stern :) Opening issues for these
occurrences is indeed very helpful and welcome. In fact, I'm reopening this
very one because there really ought to be a non-blocking getcwd in Lwt_unix,
it's clearly an oversight. It's just hard to promise when it'll happen exactly :)
(while getcwd should represent little effort, it's sensible to wait a bit
before the next maintenance releases, lest we end up causing "release
fatigue", for there are external costs involved: OPAM repository maintenance,
the OPAM weather service, mailing list announcements, etc.)

--
Mauricio Fernández

[rgrinberg]

@agarwal why is async's getcwd blocking anyway? I checked the manpage for getcwd and I don't see any mention of blocking semantics. Also, libuv's getcwd wrapper doesn't use a callback either.

[mfp]

@rgrinberg Did you see the getcwd implementation aantron linked to? The first thing it does is open(".", O_RDONLY) followed by fstat on the fd, which looks like it could indeed block (and for a really long time on NFS!). It might not be the implementation macOS uses at the moment, but it does show that some implementations out there can block.

[aantron]

@kennetpostigo To agree with the above, opening an issue for each one is fine, indeed preferred. Presumably, there aren't too many such ambiguous calls, but in any case it will help us to clarify and keep track of these questions, and the improvements that need to be made.

The performance of the three non-blocking approaches mentioned so far (Lwt_preemptive, uwt, and Async), should be pretty much the same. I don't know how the Async one is implemented, but uwt and Lwt_preemptive both farm the work out to a thread pool, and I would guess that Async does the same thing. And, when we bind getcwd in Lwt, Lwt will also be internally farming the work out to a thread pool.

We would probably do a release with getcwd in a few months, perhaps three – that would be the next "ordinary" release of Lwt. We can, of course, do an immediate release if there is an emergency. Also, if this kind of thing keeps happening, we can switch to more frequent releases somehow.

I know that writing let lwt_blah = Lwt_preemptive Unix.blah in the meantime will be a slight pain, but hopefully it's not too awful. FWIW last time I used Lwt and getcwd in one codebase (long before I was working on Lwt), I just assumed it was non-blocking and used Sys.getcwd directly. I never ran into any problems, but of course you may have an entirely different environment and different things to consider.

Also, +1 for reopening this issue, so we don't somehow forget to take care of it :)

[aantron]

Also, besides the man pages and source code, you could glance at what's bound in libuv. If it's bound, there's probably at least one platform on which the corresponding "direct" call might block. For example, see uv_cwd. This is clearly a bit of a pain, so we also want to address it as described in preceding comments. But it might be easier than reading source for now.

[agarwal]

@agarwal why is async's getcwd blocking anyway?

@aantron's previous comment gives some possible answers. That was pretty thorough. I don't know more than that.

[aantron]

Actually, @rgrinberg has a point. The libuv uv_cwd seems to assume that the getcwd(3) call won't block. It's possible I either was looking at the wrong code, or maybe it just blocks so rarely that they haven't yet dealt with it in libuv. I guess this calls for eventual careful reading and/or making an effort to actually get it to block on some system. Maybe we can just ask the libuv developers, or the authors of that code, for their expertise.

[aantron]

On top of that, looking at the actual uv_cwd API, the API is inherently synchronous. However, uwt (apparently) runs uv_cwd inside a thread, so perhaps @fdopen can comment on whether uv_cwd should itsel have been asynchronous or not. Async runs getcwd in a thread (also apparently).

[mfp]

@aantron the code you found looks like a smoking gun to me. It would take much more effort to analyze exhaustively all getcwd implementations in use (or reach out to get the info) to verify that none does actually block (plus there's a small chance that could change later) than to add the corresponding job to Lwt_unix. And it's not like getcwd is performance-critical either (or so I hope ;).

[fdopen]

There are several blocking functions inside http://docs.libuv.org/en/v1.x/misc.html . They are intended to be called synchronously at process initialization.

The current working directory is a process global resource, it is shared by all threads. If you have multiple parallel I/O threads and one (or more) of them might call chdir, you are asking for trouble. You would have to carefully synchronize (Lwt_mutex,etc.) everything that involves relative paths and function calls likerealpath, chdir, ... Calling getcwd won’t help at all. Another thread could change it before your current background thread can even report the result.

So the usual advice is to call getcwd/chdir only during startup and work thereafter with either absolute paths (preferable on windows) or paths relative to a never changing CWD. If you follow this rule, you don’t need asynchronous getcwd and chdir functions.

[mfp]

On Thu, Apr 20, 2017 at 04:01:19PM -0700, Andreas Hauptmann wrote:

So the usual advice is to call getcwd/chdir only during startup and work
thereafter with either absolute paths (preferable on windows) or paths
relative to a never changing CWD. If you follow this rule, you don’t need
asynchronous getcwd and chdir functions.

That is sensible advice for applications, but library code lacks precise
control: if getcwd is called as a side-effect of module initialization, it
will probably be too early (and it is messy and error-prone to control the
initialization order of different modules); if done later, even if only called
once, by that time there might already be other concurrent promises. (You
could of course argue that well-written libs that need the CWD should accept
it as a parameter, but I'm not sure that's something Lwt should impose on
them, or conversely if that's something Lwt should punish end-users for when
not done.)

We cannot completely rule out the possibility of a program doing e.g. network
I/O in parallel with a single promise performing filesystem operations (which
therefore has full "ownership" of the CWD, so to speak). Also, while it is
generally understood that it's a bad idea to chdir(2) throughout the life of
the process, it's not so obvious that getcwd(3) can be blocking, so even
disciplined, "safe" code that doesn't chdir could be bitten unexpectedly by a
seemingly harmless function.

--
Mauricio Fernández

[raphael-proust]

I'll get a shot at this next week.

[raphael-proust]

So my C is way more rusty than I originally thought. I managed to put some C code together and to create tests that actually call the C code. But I always get ERANGE: “the length of the absolute pathname of the current working directory, including the terminating null byte, exceeds size bytes.”

Currently, size is basically hardcoded to 1024 and the path is definitely shorter than that. I think I'm doing something wrong with pointers and I'm somehow passing a 0-length buffer.

Any pointers? (pun not intended)

/* getcwd */

struct job_getcwd {
    struct lwt_unix_job job;
    char buf[1024] ;
    char* result;
    int error_code;
};

static void worker_getcwd(struct job_getcwd *job)
{
    job->result = getcwd(job->buf, sizeof(job->buf));
    job->error_code = errno;
}

static value result_getcwd(struct job_getcwd *job)
{
    LWT_UNIX_CHECK_JOB(job, job->result != 0, "getcwd");
    value result = caml_copy_string(job->result);
    lwt_unix_free_job(&job->job);
    return result;
}

CAMLprim value lwt_unix_getcwd_job()
{
    LWT_UNIX_INIT_JOB(job, getcwd, 0);
    return lwt_unix_alloc_job(&job->job);
}

[aantron]

I don't get the same problem with a pared-down version of this code (on macOS):

#include <errno.h>
#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>

struct job_getcwd {
    char buf[1024];
    char* result;
    int error_code;
};

int main()
{
    struct job_getcwd   the_job;
    struct job_getcwd   *job = &the_job;

    job->result = getcwd(job->buf, sizeof(job->buf));
    job->error_code = errno;

    if (job->result == NULL) {
        printf("getcwd returned NULL\n");
        return EXIT_FAILURE;
    }

    if (job->error_code != 0) {
        printf("errno is %i\n", job->error_code);
        return EXIT_FAILURE;
    }

    printf("%s\n", job->result);

    return EXIT_SUCCESS;
}

Does this work on your system? Can you confirm that getcwd is actually failing inside the worker thread (in worker_getcwd)? That would rule out problems with the Lwt machinery getting errno back to the main thread correctly.

[raphael-proust]

That minimal code works on my system.

---

In trying to fix things and understand what is broken, I now get a different error: ENOENT (the current working directory has been unlinked).

I am certain the current directory is not unlinked on my side. Is the lwt test suite executed in a temporary directory that gets unlinked before it finishes? AFAICT from setup.ml this is not the case (test_working_directory = None; for all test suites).

---

I have written two tests:

    test "getcwd"
    (fun () ->
      Lwt_unix.getcwd () >>= fun (_:string) ->
      Lwt.return_true
    );
    test "chdir and getcwd"
    (fun () ->
      Lwt_unix.getcwd () >>= fun here ->
      Lwt_unix.chdir "/" >>= fun () ->
      Lwt_unix.getcwd () >>= fun root ->
      Lwt_unix.chdir here >>= fun () ->
      Lwt.return (root = "/")
    );

(Admittedly, the second test should ultimately be removed. If the second getcwd or chdir fail, then the rest of the tests execute in / which is not somewhere people want tests ran. Also, it causes EACCESS and such.)

Both of them fail:

Running test "getcwd" from suite "lwt_unix"
Test "getcwd" from suite "lwt_unix" failed. It raised: "Unix.Unix_error(Unix.ENOENT, \"getcwd\", \"\")".
Running test "chdir and getcwd" from suite "lwt_unix"
Test "chdir and getcwd" from suite "lwt_unix" failed. It raised: "Unix.Unix_error(Unix.ENOENT, \"getcwd\",\"\")".

I added printfs to the code as follows:

static void worker_getcwd(struct job_getcwd *job)
{
    job->result = getcwd(job->buf, sizeof(job->buf));
    job->error_code = errno;

    if (job->result == NULL) {
        printf("getcwd returned NULL\n");
    }

    if (job->error_code != 0) {
        printf("errno is %i\n", job->error_code);
    }

    job->error_code = errno;
}

static value result_getcwd(struct job_getcwd *job)
{
    printf("checking getcwd job result\n");
    LWT_UNIX_CHECK_JOB(job, job->error_code != 0, "getcwd");
    value result = caml_copy_string(job->result);
    lwt_unix_free_job(&job->job);
    return result;
}

CAMLprim value lwt_unix_getcwd_job()
{
    printf("initialising getcwd job\n");
    LWT_UNIX_INIT_JOB(job, getcwd, 0);
    return lwt_unix_alloc_job(&job->job);
}

And I observe these writes:

initialising getcwd job
checking getcwd job result
initialising getcwd job
errno is 2
checking getcwd job result

So the first test seems to be doing ok (result is set and errno is zero) but still fails. In the second errno is 2 (ENOENT on my system).

---

In retrospect, this was probably not the best issue for me to tackle due to too old C skills. I'll keep trying to fix it this week.

[aantron]

I think the problem is on this line:

LWT_UNIX_CHECK_JOB(job, job->error_code != 0, "getcwd");

the test should be job->result == NULL. IIRC if a system call succeeds, it does not clear errno back to zero, so this code (and the rest of the C code in Lwt that reads errno) is always picking up the errno of whatever system call was last to fail. So errno can't be used as a test for failure.

This stuff is really not clear, and it's the fault of Lwt, not your C skills. I had to look up the macro to figure out which way the test goes, and the thing with errno has bitten countless people in history. Should probably add notes about this to CONTRIBUTING.md and link them somehow, and also keep this in mind if refactoring the C code in the future. There is unfortunately no obvious place to document this kind of stuff in lwt_unix_unix.h, because it's such a huge file, but maybe we should carefully document bind (or getcwd?) as some kind of model, and refer people to it.

[raphael-proust]

Oh… So I thought of that at some point but I had forgotten to change the comparison operator (so I ended up testing job->result != NULL which…)

It works now. I'll make the diff prettier and do a commit, then there are a few improvements to make (like better tests and use of MAX_PATH or something like that instead of hard-coded buf size).

Also, I noticed some errors in the make test system: it doesn't redo everything. I had to make clean all test. I'm opening a separate issue.

[aantron]

FYI, concerning C skills, I also didn't notice the incorrect condition either time, even though you showed the code. So, yes, this stuff is not programmer-friendly (and hence the #396 to avoid dealing with it again – thanks).