Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EINTR-based signals #1128

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 3 additions & 8 deletions asmrun/signals_asm.c
Expand Up @@ -96,19 +96,14 @@ DECLARE_SIGNAL_HANDLER(handle_signal)
signal(sig, handle_signal);
#endif
if (sig < 0 || sig >= NSIG) return;
if (caml_try_leave_blocking_section_hook ()) {
caml_execute_signal(sig, 1);
caml_enter_blocking_section_hook();
} else {
caml_record_signal(sig);
caml_record_signal(sig);
/* Some ports cache [caml_young_limit] in a register.
Use the signal context to modify that register too, but only if
we are inside OCaml code (not inside C code). */
#if defined(CONTEXT_PC) && defined(CONTEXT_YOUNG_LIMIT)
if (Is_in_code_area(CONTEXT_PC))
CONTEXT_YOUNG_LIMIT = (context_reg) caml_young_limit;
if (Is_in_code_area(CONTEXT_PC))
CONTEXT_YOUNG_LIMIT = (context_reg) caml_young_limit;
#endif
}
errno = saved_errno;
}

Expand Down
41 changes: 25 additions & 16 deletions byterun/caml/io.h
Expand Up @@ -54,9 +54,6 @@ struct channel {

enum {
CHANNEL_FLAG_FROM_SOCKET = 1, /* For Windows */
#if defined(NATIVE_CODE) && defined(WITH_SPACETIME)
CHANNEL_FLAG_BLOCKING_WRITE = 2,
#endif
};

/* For an output channel:
Expand All @@ -66,35 +63,47 @@ enum {
*/

/* Functions and macros that can be called from C. Take arguments of
type struct channel *. No locking is performed. */

#define caml_putch(channel, ch) do{ \
if ((channel)->curr >= (channel)->end) caml_flush_partial(channel); \
*((channel)->curr)++ = (ch); \
}while(0)

#define caml_getch(channel) \
((channel)->curr >= (channel)->max \
? caml_refill(channel) \
: (unsigned char) *((channel)->curr)++)
type struct channel *. */

CAMLextern struct channel * caml_open_descriptor_in (int);
CAMLextern struct channel * caml_open_descriptor_out (int);
CAMLextern void caml_close_channel (struct channel *);
CAMLextern int caml_channel_binary_mode (struct channel *);
CAMLextern value caml_alloc_channel(struct channel *chan);

CAMLextern int caml_flush_partial (struct channel *);
/* High-level I/O operations.
These functions raise exceptions if errors occur.
They may run signal handlers.
The provided channel must *not* be locked:
these functions lock the channel themselves. */

CAMLextern void caml_flush (struct channel *);
CAMLextern void caml_putch(struct channel*, unsigned char ch);
CAMLextern void caml_putword (struct channel *, uint32_t);
CAMLextern int caml_putblock (struct channel *, char *, intnat);
CAMLextern void caml_really_putblock (struct channel *, char *, intnat);

CAMLextern unsigned char caml_refill (struct channel *);
CAMLextern unsigned char caml_getch(struct channel*);
CAMLextern uint32_t caml_getword (struct channel *);
CAMLextern int caml_getblock (struct channel *, char *, intnat);
CAMLextern intnat caml_really_getblock (struct channel *, char *, intnat);

/* Low-level I/O operations.
These functions return an error code (0 for success).
They never raise exceptions nor run signal handlers,
but may return EINTR if a signal arrives.
(In such cases, the caller should unlock the channel
and run caml_process_pending_signals).
The provided channel must be locked by the caller. */

typedef int io_result;

CAMLextern io_result caml_try_flush (struct channel *);
CAMLextern io_result caml_try_putblock (struct channel *, char *, intnat,
int* written);
CAMLextern io_result caml_try_getblock (struct channel *, char *, intnat,
int* read);

/* Extract a struct channel * from the heap object representing it */

#define Channel(v) (*((struct channel **) (Data_custom_val(v))))
Expand Down
13 changes: 7 additions & 6 deletions byterun/caml/osdeps.h
Expand Up @@ -22,22 +22,23 @@

#include "misc.h"
#include "memory.h"
#include "io.h"

/* Read at most [n] bytes from file descriptor [fd] into buffer [buf].
[flags] indicates whether [fd] is a socket
(bit [CHANNEL_FLAG_FROM_SOCKET] is set in this case, see [io.h]).
(This distinction matters for Win32, but not for Unix.)
Return number of bytes read.
In case of error, raises [Sys_error] or [Sys_blocked_io]. */
extern int caml_read_fd(int fd, int flags, void * buf, int n);
Returns error code (0 on success, or positive error code)
If successful, stores number of bytes read into *nread. */
extern io_result caml_read_fd(int fd, int flags, void * buf, int n, int* nread);

/* Write at most [n] bytes from buffer [buf] onto file descriptor [fd].
[flags] indicates whether [fd] is a socket
(bit [CHANNEL_FLAG_FROM_SOCKET] is set in this case, see [io.h]).
(This distinction matters for Win32, but not for Unix.)
Return number of bytes written.
In case of error, raises [Sys_error] or [Sys_blocked_io]. */
extern int caml_write_fd(int fd, int flags, void * buf, int n);
Return error code (0 on success or positive error code)
If successful, stores number of bytes written into *nwritten. */
extern io_result caml_write_fd(int fd, int flags, void * buf, int n, int* nwritten);

/* Decompose the given path into a list of directories, and add them
to the given table. */
Expand Down
1 change: 1 addition & 0 deletions byterun/caml/signals.h
Expand Up @@ -51,6 +51,7 @@ CAMLextern void (* volatile caml_async_action_hook)(void);

CAMLextern void caml_enter_blocking_section (void);
CAMLextern void caml_leave_blocking_section (void);
CAMLextern void caml_leave_blocking_section_nosig (void);

#ifdef __cplusplus
}
Expand Down
8 changes: 6 additions & 2 deletions byterun/caml/sys.h
Expand Up @@ -26,8 +26,12 @@ extern "C" {

#define NO_ARG Val_int(0)

CAMLextern void caml_sys_error (value);
CAMLextern void caml_sys_io_error (value);
CAMLnoreturn_start
CAMLextern void caml_sys_error (value)
CAMLnoreturn_end;
CAMLnoreturn_start
CAMLextern void caml_sys_io_error (value)
CAMLnoreturn_end;
CAMLextern double caml_sys_time_unboxed(value);
CAMLextern void caml_sys_init (char * exe_name, char ** argv);
CAMLextern value caml_sys_exit (value);
Expand Down
2 changes: 0 additions & 2 deletions byterun/extern.c
Expand Up @@ -684,9 +684,7 @@ CAMLprim value caml_output_value(value vchan, value v, value flags)
CAMLparam3 (vchan, v, flags);
struct channel * channel = Channel(vchan);

Lock(channel);
caml_output_val(channel, v, flags);
Unlock(channel);
Comment on lines -687 to -689
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand well, this means that while we took the lock only once before this PR, we take the lock once for every externed block. I can see two problems with that:

  • Atomicity is no longer guaranteed, and the unmarshalled data can be interleaved with whatever a signal handler decided to write. But this is true in general for anything in this PR, so I don't think there is anything we can do.
  • Performance: are we sure that the repeated lock acquisition/release has negligible cost?

CAMLreturn (Val_unit);
}

Expand Down
4 changes: 0 additions & 4 deletions byterun/intern.c
Expand Up @@ -755,9 +755,7 @@ CAMLprim value caml_input_value(value vchan)
struct channel * chan = Channel(vchan);
CAMLlocal1 (res);

Lock(chan);
res = caml_input_val(chan);
Unlock(chan);
CAMLreturn (res);
}

Expand All @@ -769,9 +767,7 @@ CAMLprim value caml_input_value_to_outside_heap(value vchan)
struct channel * chan = Channel(vchan);
CAMLlocal1 (res);

Lock(chan);
res = caml_input_val_core(chan, 1);
Unlock(chan);
CAMLreturn (res);
}

Expand Down