From 8ee717520fa6c487b03fa80df17bcab55e55529a Mon Sep 17 00:00:00 2001 From: Matthew Flatt Date: Wed, 2 Sep 2015 16:14:20 -0600 Subject: [PATCH] fix `sync` when resumed after a break exception This repair adjusts the bug fix of commit 769ad3e98. That older commit ensured that `sync/enable-break` doesn't both break and accept a channel message or semaphore wait. But it effectively disables those actions if the break is continued. Instead of (partially!) ending the `sync` get out of semaphore and channel queues so that no event can be selected during the break, and then get back in line if the break is continued. --- pkgs/racket-test-core/tests/racket/sync.rktl | 32 ++++++++++++ racket/src/racket/src/schpriv.h | 2 + racket/src/racket/src/sema.c | 5 ++ racket/src/racket/src/struct.c | 7 +++ racket/src/racket/src/thread.c | 52 +++++++++++++++++++- 5 files changed, 96 insertions(+), 2 deletions(-) diff --git a/pkgs/racket-test-core/tests/racket/sync.rktl b/pkgs/racket-test-core/tests/racket/sync.rktl index ea4dbc47c5..aeb6627bb0 100644 --- a/pkgs/racket-test-core/tests/racket/sync.rktl +++ b/pkgs/racket-test-core/tests/racket/sync.rktl @@ -1403,6 +1403,38 @@ (break-thread t) (test t sync t)) +;; ---------------------------------------- +;; result from a break during `sync` + +(let () + (define (try wrap val) + (for ([tsync (list sync sync/enable-break)]) + (define ch (make-channel)) + (define got #f) + (define t (thread (lambda () + (define nack #f) + (call-with-exception-handler + (lambda (exn) + (test #f sync/timeout 0 nack) + (if (exn:break? exn) + ((exn:break-continuation exn)) + exn)) + (lambda () + (set! got (tsync (nack-guard-evt + (lambda (n) + (set! nack n) + (wrap ch)))))))))) + (sync (system-idle-evt)) + (break-thread t) + (sync (system-idle-evt)) + (channel-put ch val) + (sync t) + (test val values got))) + + (try values 'ok-channel) + (try (lambda (c) (choice-evt c (alarm-evt (+ 10000 (current-milliseconds))))) + 'ok-channel+alarm)) + ;; ---------------------------------------- (report-errs) diff --git a/racket/src/racket/src/schpriv.h b/racket/src/racket/src/schpriv.h index a44a82d6b3..12580e462d 100644 --- a/racket/src/racket/src/schpriv.h +++ b/racket/src/racket/src/schpriv.h @@ -787,6 +787,7 @@ void scheme_syncing_needs_wakeup(struct Syncing *s, void *fds); void scheme_escape_during_sync(struct Syncing *syncing); Scheme_Object *scheme_syncing_result(struct Syncing *syncing, int tailok); +struct Syncing *scheme_replace_evt_get(Scheme_Object *active_replace); struct Syncing *scheme_replace_evt_nack(Scheme_Object *active_replace); struct Syncing *scheme_replace_evt_needs_wakeup(Scheme_Object *o); @@ -1992,6 +1993,7 @@ Scheme_Object *scheme_do_chaperone_evt(const char*, int, int, Scheme_Object *arg extern Scheme_Object *scheme_always_ready_evt; void scheme_get_outof_line(Scheme_Channel_Syncer *ch_w); +void scheme_get_back_into_line(Scheme_Channel_Syncer *ch_w); void scheme_post_syncing_nacks(Syncing *syncing); int scheme_try_channel_get(Scheme_Object *ch); diff --git a/racket/src/racket/src/sema.c b/racket/src/racket/src/sema.c index 9248347b89..49ac297139 100644 --- a/racket/src/racket/src/sema.c +++ b/racket/src/racket/src/sema.c @@ -544,6 +544,11 @@ void scheme_get_outof_line(Scheme_Channel_Syncer *ch_w) get_outof_line((Scheme_Sema *)ch_w->obj, ch_w); } +void scheme_get_back_into_line(Scheme_Channel_Syncer *ch_w) +{ + get_into_line((Scheme_Sema *)ch_w->obj, ch_w); +} + static int try_channel(Scheme_Sema *sema, Syncing *syncing, int pos, Scheme_Object **result) { if (SCHEME_CHANNELP(sema)) { diff --git a/racket/src/racket/src/struct.c b/racket/src/racket/src/struct.c index 8007b205f2..dd43c2836d 100644 --- a/racket/src/racket/src/struct.c +++ b/racket/src/racket/src/struct.c @@ -4191,6 +4191,13 @@ Syncing *scheme_replace_evt_nack(Scheme_Object *o) return s; } +Syncing *scheme_replace_evt_get(Scheme_Object *o) +{ + Active_Replace_Evt *a = (Active_Replace_Evt *)o; + + return a->syncing; +} + /*========================================================================*/ /* struct op maker */ /*========================================================================*/ diff --git a/racket/src/racket/src/thread.c b/racket/src/racket/src/thread.c index 74934e86a3..7f47807b87 100644 --- a/racket/src/racket/src/thread.c +++ b/racket/src/racket/src/thread.c @@ -436,6 +436,7 @@ static void suspend_thread(Scheme_Thread *p); static int check_sleep(int need_activity, int sleep_now); static int syncing_ready(Syncing *syncing, Scheme_Schedule_Info *sinfo); +static void get_outof_or_into_lines(Syncing *syncing, int get_out); static void remove_thread(Scheme_Thread *r); static void exit_or_escape(Scheme_Thread *p); @@ -4606,8 +4607,10 @@ static void raise_break(Scheme_Thread *p) p->external_break = 0; if (p->blocker && (p->block_check == (Scheme_Ready_Fun)syncing_ready)) { - /* Get out of lines for channels, etc., before calling a break exn handler. */ - scheme_post_syncing_nacks((Syncing *)p->blocker); + /* Get out of lines for channels, etc., before calling a break exn handler. + This is only strictly necessary for `sync/enable-break`, which wants + to provide a sync-or-break guarantee, but we do it always for consistency. */ + get_outof_or_into_lines((Syncing *)p->blocker, 1); } save_thread_schedule_state(p, &ssr, 0); @@ -4626,6 +4629,11 @@ static void raise_break(Scheme_Thread *p) /* Continue from break... */ restore_thread_schedule_state(p, &ssr, 0); + + if (p->blocker && (p->block_check == (Scheme_Ready_Fun)syncing_ready)) { + /* Get back into lines for channels, etc. */ + get_outof_or_into_lines((Syncing *)p->blocker, 0); + } } static void escape_to_kill(Scheme_Thread *p) @@ -7002,6 +7010,46 @@ static void post_syncing_nacks(Syncing *syncing, int as_escape) } while (syncing); } +static void get_outof_or_into_lines(Syncing *syncing, int get_out) +{ + int i, c; + Scheme_Object *syncs = NULL; + Syncing *next; + + if (syncing->result) { + /* already done, so no need to adjust lines */ + return; + } + + do { + if (syncing->set) { + c = syncing->set->argc; + + for (i = 0; i < c; i++) { + if (SAME_TYPE(SCHEME_TYPE(syncing->set->argv[i]), scheme_channel_syncer_type)) { + if (get_out) + scheme_get_outof_line((Scheme_Channel_Syncer *)syncing->set->argv[i]); + else + scheme_get_back_into_line((Scheme_Channel_Syncer *)syncing->set->argv[i]); + } + else if (SAME_TYPE(SCHEME_TYPE(syncing->set->argv[i]), scheme_active_replace_evt_type)) { + /* Handle active_replace_evt specially to avoid stack overflow: */ + next = scheme_replace_evt_get(syncing->set->argv[i]); + if (next) + syncs = scheme_make_raw_pair((Scheme_Object *)next, syncs); + } + } + } + + if (!syncs) + syncing = NULL; + else { + syncing = (Syncing *)SCHEME_CAR(syncs); + syncs = SCHEME_CDR(syncs); + } + } while (syncing); +} + void scheme_post_syncing_nacks(Syncing *syncing) { post_syncing_nacks(syncing, 0);