[dart:io] Namespaces for file IO

[dart:io] Namespaces for file IO

Fuchsia requires the ability to sandbox Isolates w.r.t. file IO.
When a new Isolate starts, Fuchsia will pass the Isolate an object
called a namespace. We can translate the namespace object into a
file descriptor suitable for passing to the *at() family of
POSIX file system calls. The file system calls will then
have visibility only into the specified namespace.

We also plumb Namespaces through on all the other platforms as well to
make the change easier to test and so that in the future we can
implement e.g. per-isolate cwds.

This change adds a new internal class to dart:io called _Namespace,
which is implemented in a patch file. See:

sdk/lib/io/namespace_impl.dart
runtime/bin/namespace_patch.dart

The embedder can set up a non-default namespace by calling
_Namespace._setupNamespace during Isolate setup.

Instances of _Namespace have a native field that holds a pointer
to a native Namespace object. See:

runtime/bin/namespace.h

Calls from e.g. file_impl.dart are now also passed a
_Namespace object. The implementations in e.g. file.cc and
file_linux.cc then extract the namespace, and use it to compute a
file descriptor and path suitable for passing to e.g. openat().

related US-313

R=asiva@google.com, rmacnak@google.com

Committed: https://github.com/dart-lang/sdk/commit/d0295c873c906567d0739a906950874380bafdbb

[zra, 2017-08-28 21:23:06]

Description was changed from

==========
[dart:io] Namespaces for file IO

Fuchsia requires the ability to sandbox Isolates w.r.t. file IO.
When a new Isolate starts, Fuchsia will pass the Isolate an object
called a namespace. We can translate the namespace object into a
file descriptor suitable for passing to the *at() family of
POSIX file system calls. The file system calls will then
have visibility only into the specified namespace.

We also plumb Namespaces through on all the other platforms as well to
make the change easier to test and so that in the future we can
implement e.g. per-isolate cwds.

fixes US-313
==========

to

==========
[dart:io] Namespaces for file IO

Fuchsia requires the ability to sandbox Isolates w.r.t. file IO.
When a new Isolate starts, Fuchsia will pass the Isolate an object
called a namespace. We can translate the namespace object into a
file descriptor suitable for passing to the *at() family of
POSIX file system calls. The file system calls will then
have visibility only into the specified namespace.

We also plumb Namespaces through on all the other platforms as well to
make the change easier to test and so that in the future we can
implement e.g. per-isolate cwds.

This change adds a new internal class to dart:io called _Namespace,
which is implemented in a patch file. See:

sdk/lib/io/namespace_impl.dart
runtime/bin/namespace_patch.dart

The embedder can set up a non-default namespace by calling
_Namespace._setupNamespace during Isolate setup.

Instances of _Namespace have a native field that holds a pointer
to a native Namespace object. See:

runtime/bin/namespace.h

Calls from e.g. file_impl.dart are now also passed a
_Namespace object. The implementations in e.g. file.cc and
file_linux.cc then extract the namespace, and use it to compute a
file descriptor and path suitable for passing to e.g. openat().

fixes US-313
==========

[zra, 2017-08-28 21:24:47]

zra@google.com changed reviewers:
+ asiva@google.com, rmacnak@google.com

[siva, 2017-08-29 16:47:39]

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace.cc
File runtime/bin/namespace.cc (right):

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace.cc#ne...
runtime/bin/namespace.cc:22: Namespace* namespc =
reinterpret_cast<Namespace*>(peer);
ASSERT(namespc != NULL);

or

if (namespc != NULL) {
  namespc->Release();
}

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace.cc#ne...
runtime/bin/namespace.cc:26: static void AssertIsNamespace(Dart_Handle
namespc_obj) {
Why not change this into
#if defined(DEBUG)
static bool IsNamespace(Dart_Handle namespc_obj) {
  ...
  ...
  ...
  return isInstance;
}
#endif

then you could turn all the call points into
ASSERT(IsNamespace(namespc_obj));

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace.cc#ne...
runtime/bin/namespace.cc:100: Dart_SetIntegerReturnValue(args, namespc_pointer);
Why not just
Dart_SetIntegerReturnValue(args, reinterpret_cast<intptr_t>(namespc));

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace.h
File runtime/bin/namespace.h (right):

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace.h#new...
runtime/bin/namespace.h:18: // Assumes napespc is a value that can be directly
used as namespc_.
namespc

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace.h#new...
runtime/bin/namespace.h:24: // Gives a safe defautl value for namespc_ for the
standalone Dart VM.
default

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace.h#new...
runtime/bin/namespace.h:78: explicit NamespaceScope(Namespace* namespc, const
char* path);
Why explicit for this constructor, seems to have 2 parameters?

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace_andro...
File runtime/bin/namespace_android.cc (right):

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace_andro...
runtime/bin/namespace_android.cc:21: const int fd =
TEMP_FAILURE_RETRY(open(path, O_RDONLY));
const intptr_t fd = static_cast<intptr_t>(TEMP....);
so that the call to Namespace constructor gets the correct type.

[rmacnak, 2017-08-29 18:00:13]

I get a failure on standalone/io/file_test:

FileSystemException: Failed to set file modification time, path =
'/tmp/dart_fileJEHLLG/set_last_modified_sync_test' (OS Error: Invalid argument,
errno = 22)
#0      _File.setLastModifiedSync (dart:io/file_impl.dart:457)

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/dartutils.h
File runtime/bin/dartutils.h (right):

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/dartutils.h#new...
runtime/bin/dartutils.h:133: static Dart_Handle SetupIOLibrary(const char*
namespc,
namespc_path

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/file_android.cc
File runtime/bin/file_android.cc (right):

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/file_android.cc...
runtime/bin/file_android.cc:533: return syscall(__NR_readlinkat, dirfd,
pathname, buf, bufsize);
Maybe note the Android version that adds this to libc.

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/main.cc
File runtime/bin/main.cc (right):

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/main.cc#newcode...
runtime/bin/main.cc:1339: #if defined(HOST_OS_LINUX)
or Android

https://codereview.chromium.org/3007703002/diff/1/sdk/lib/io/namespace_impl.dart
File sdk/lib/io/namespace_impl.dart (right):

https://codereview.chromium.org/3007703002/diff/1/sdk/lib/io/namespace_impl.d...
sdk/lib/io/namespace_impl.dart:7: // Each Isolate may run in a different
"namespace".
, which provides the scope in which file paths are resolved.

[zra, 2017-08-29 20:22:52]

Thanks! Fixed the test failure.

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/dartutils.h
File runtime/bin/dartutils.h (right):

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/dartutils.h#new...
runtime/bin/dartutils.h:133: static Dart_Handle SetupIOLibrary(const char*
namespc,
On 2017/08/29 18:00:12, rmacnak wrote:
> namespc_path

Done.

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/file_android.cc
File runtime/bin/file_android.cc (right):

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/file_android.cc...
runtime/bin/file_android.cc:533: return syscall(__NR_readlinkat, dirfd,
pathname, buf, bufsize);
On 2017/08/29 18:00:12, rmacnak wrote:
> Maybe note the Android version that adds this to libc.

Done.

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/main.cc
File runtime/bin/main.cc (right):

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/main.cc#newcode...
runtime/bin/main.cc:1339: #if defined(HOST_OS_LINUX)
On 2017/08/29 18:00:13, rmacnak wrote:
> or Android

Done.

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace.cc
File runtime/bin/namespace.cc (right):

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace.cc#ne...
runtime/bin/namespace.cc:22: Namespace* namespc =
reinterpret_cast<Namespace*>(peer);
On 2017/08/29 16:47:39, siva wrote:
> ASSERT(namespc != NULL);
> 
> or
> 
> if (namespc != NULL) {
>   namespc->Release();
> }

Done.

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace.cc#ne...
runtime/bin/namespace.cc:26: static void AssertIsNamespace(Dart_Handle
namespc_obj) {
On 2017/08/29 16:47:39, siva wrote:
> Why not change this into
> #if defined(DEBUG)
> static bool IsNamespace(Dart_Handle namespc_obj) {
>   ...
>   ...
>   ...
>   return isInstance;
> }
> #endif
> 
> then you could turn all the call points into
> ASSERT(IsNamespace(namespc_obj));

Done.

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace.cc#ne...
runtime/bin/namespace.cc:100: Dart_SetIntegerReturnValue(args, namespc_pointer);
On 2017/08/29 16:47:39, siva wrote:
> Why not just
> Dart_SetIntegerReturnValue(args, reinterpret_cast<intptr_t>(namespc));

Done.

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace.h
File runtime/bin/namespace.h (right):

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace.h#new...
runtime/bin/namespace.h:18: // Assumes napespc is a value that can be directly
used as namespc_.
On 2017/08/29 16:47:39, siva wrote:
> namespc

Done.

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace.h#new...
runtime/bin/namespace.h:24: // Gives a safe defautl value for namespc_ for the
standalone Dart VM.
On 2017/08/29 16:47:39, siva wrote:
> default

Done.

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace.h#new...
runtime/bin/namespace.h:78: explicit NamespaceScope(Namespace* namespc, const
char* path);
On 2017/08/29 16:47:39, siva wrote:
> Why explicit for this constructor, seems to have 2 parameters?

Oops. Leftover after a refactor. Removed.

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace_andro...
File runtime/bin/namespace_android.cc (right):

https://codereview.chromium.org/3007703002/diff/1/runtime/bin/namespace_andro...
runtime/bin/namespace_android.cc:21: const int fd =
TEMP_FAILURE_RETRY(open(path, O_RDONLY));
On 2017/08/29 16:47:39, siva wrote:
> const intptr_t fd = static_cast<intptr_t>(TEMP....);
> so that the call to Namespace constructor gets the correct type.

Done.

https://codereview.chromium.org/3007703002/diff/1/sdk/lib/io/namespace_impl.dart
File sdk/lib/io/namespace_impl.dart (right):

https://codereview.chromium.org/3007703002/diff/1/sdk/lib/io/namespace_impl.d...
sdk/lib/io/namespace_impl.dart:7: // Each Isolate may run in a different
"namespace".
On 2017/08/29 18:00:13, rmacnak wrote:
> , which provides the scope in which file paths are resolved.

Done.

https://codereview.chromium.org/3007703002/diff/20001/runtime/bin/file_linux.cc
File runtime/bin/file_linux.cc (right):

https://codereview.chromium.org/3007703002/diff/20001/runtime/bin/file_linux....
runtime/bin/file_linux.cc:455: t->tv_nsec = (millis - (t->tv_sec *
kMillisecondsPerSecond)) * 1000L;
Test failure was caused by a typo here. I had t->tv_nsec instead of t->tv_sec.

[zra, 2017-08-29 20:23:09]

Description was changed from

==========
[dart:io] Namespaces for file IO

Fuchsia requires the ability to sandbox Isolates w.r.t. file IO.
When a new Isolate starts, Fuchsia will pass the Isolate an object
called a namespace. We can translate the namespace object into a
file descriptor suitable for passing to the *at() family of
POSIX file system calls. The file system calls will then
have visibility only into the specified namespace.

We also plumb Namespaces through on all the other platforms as well to
make the change easier to test and so that in the future we can
implement e.g. per-isolate cwds.

This change adds a new internal class to dart:io called _Namespace,
which is implemented in a patch file. See:

sdk/lib/io/namespace_impl.dart
runtime/bin/namespace_patch.dart

The embedder can set up a non-default namespace by calling
_Namespace._setupNamespace during Isolate setup.

Instances of _Namespace have a native field that holds a pointer
to a native Namespace object. See:

runtime/bin/namespace.h

Calls from e.g. file_impl.dart are now also passed a
_Namespace object. The implementations in e.g. file.cc and
file_linux.cc then extract the namespace, and use it to compute a
file descriptor and path suitable for passing to e.g. openat().

fixes US-313
==========

to

==========
[dart:io] Namespaces for file IO

Fuchsia requires the ability to sandbox Isolates w.r.t. file IO.
When a new Isolate starts, Fuchsia will pass the Isolate an object
called a namespace. We can translate the namespace object into a
file descriptor suitable for passing to the *at() family of
POSIX file system calls. The file system calls will then
have visibility only into the specified namespace.

We also plumb Namespaces through on all the other platforms as well to
make the change easier to test and so that in the future we can
implement e.g. per-isolate cwds.

This change adds a new internal class to dart:io called _Namespace,
which is implemented in a patch file. See:

sdk/lib/io/namespace_impl.dart
runtime/bin/namespace_patch.dart

The embedder can set up a non-default namespace by calling
_Namespace._setupNamespace during Isolate setup.

Instances of _Namespace have a native field that holds a pointer
to a native Namespace object. See:

runtime/bin/namespace.h

Calls from e.g. file_impl.dart are now also passed a
_Namespace object. The implementations in e.g. file.cc and
file_linux.cc then extract the namespace, and use it to compute a
file descriptor and path suitable for passing to e.g. openat().

related US-313
==========

[rmacnak, 2017-08-29 21:18:05]

lgtm

[siva, 2017-08-29 22:13:03]

lgtm

[zra, 2017-08-30 16:34:41]

Description was changed from

==========
[dart:io] Namespaces for file IO

Fuchsia requires the ability to sandbox Isolates w.r.t. file IO.
When a new Isolate starts, Fuchsia will pass the Isolate an object
called a namespace. We can translate the namespace object into a
file descriptor suitable for passing to the *at() family of
POSIX file system calls. The file system calls will then
have visibility only into the specified namespace.

We also plumb Namespaces through on all the other platforms as well to
make the change easier to test and so that in the future we can
implement e.g. per-isolate cwds.

This change adds a new internal class to dart:io called _Namespace,
which is implemented in a patch file. See:

sdk/lib/io/namespace_impl.dart
runtime/bin/namespace_patch.dart

The embedder can set up a non-default namespace by calling
_Namespace._setupNamespace during Isolate setup.

Instances of _Namespace have a native field that holds a pointer
to a native Namespace object. See:

runtime/bin/namespace.h

Calls from e.g. file_impl.dart are now also passed a
_Namespace object. The implementations in e.g. file.cc and
file_linux.cc then extract the namespace, and use it to compute a
file descriptor and path suitable for passing to e.g. openat().

related US-313
==========

to

==========
[dart:io] Namespaces for file IO

Fuchsia requires the ability to sandbox Isolates w.r.t. file IO.
When a new Isolate starts, Fuchsia will pass the Isolate an object
called a namespace. We can translate the namespace object into a
file descriptor suitable for passing to the *at() family of
POSIX file system calls. The file system calls will then
have visibility only into the specified namespace.

We also plumb Namespaces through on all the other platforms as well to
make the change easier to test and so that in the future we can
implement e.g. per-isolate cwds.

This change adds a new internal class to dart:io called _Namespace,
which is implemented in a patch file. See:

sdk/lib/io/namespace_impl.dart
runtime/bin/namespace_patch.dart

The embedder can set up a non-default namespace by calling
_Namespace._setupNamespace during Isolate setup.

Instances of _Namespace have a native field that holds a pointer
to a native Namespace object. See:

runtime/bin/namespace.h

Calls from e.g. file_impl.dart are now also passed a
_Namespace object. The implementations in e.g. file.cc and
file_linux.cc then extract the namespace, and use it to compute a
file descriptor and path suitable for passing to e.g. openat().

related US-313

R=asiva@google.com, rmacnak@google.com

Committed:
https://github.com/dart-lang/sdk/commit/d0295c873c906567d0739a906950874380bafdbb
==========

[zra, 2017-08-30 16:34:43]

Committed patchset #4 (id:60001) manually as
d0295c873c906567d0739a906950874380bafdbb (presubmit successful).