Marge Bot pushed to branch master at Glasgow Haskell Compiler / GHC

Commits:

10 changed files:

Changes:

  • rts/RtsStartup.c
    ... ... @@ -415,8 +415,8 @@ hs_init_ghc(int *argc, char **argv[], RtsConfig rts_config)
    415 415
         traceInitEvent(dumpIPEToEventLog);
    
    416 416
         initHeapProfiling();
    
    417 417
     
    
    418
    -    /* start the virtual timer 'subsystem'. */
    
    419
    -    startTimer();
    
    418
    +    /* start the timer (after initTimer above) */
    
    419
    +    unpauseTimer();
    
    420 420
     
    
    421 421
     #if defined(RTS_USER_SIGNALS)
    
    422 422
         if (RtsFlags.MiscFlags.install_signal_handlers) {
    
    ... ... @@ -512,14 +512,12 @@ hs_exit_(bool wait_foreign)
    512 512
         }
    
    513 513
     #endif
    
    514 514
     
    
    515
    -    /* stop the ticker */
    
    516
    -    stopTimer();
    
    517
    -    /*
    
    518
    -     * it is quite important that we wait here as some timer implementations
    
    519
    -     * (e.g. pthread) may fire even after we exit, which may segfault as we've
    
    520
    -     * already freed the capabilities.
    
    515
    +    /* We rely on the guarantee that exitTimer stops the timer synchronously,
    
    516
    +     * which ensures the timer handler does not get run again after this point.
    
    517
    +     * We are about to start freeing resources used by the timer handler (like
    
    518
    +     * the capabilities, eventlog and profiling data structures).
    
    521 519
          */
    
    522
    -    exitTimer(true);
    
    520
    +    exitTimer();
    
    523 521
     
    
    524 522
         /*
    
    525 523
          * Dump the ticky counter definitions
    

  • rts/Schedule.c
    ... ... @@ -455,7 +455,7 @@ run_thread:
    455 455
             prev = setRecentActivity(ACTIVITY_YES);
    
    456 456
             if (prev == ACTIVITY_DONE_GC) {
    
    457 457
     #if !defined(PROFILING)
    
    458
    -            startTimer();
    
    458
    +            unpauseTimer();
    
    459 459
     #endif
    
    460 460
             }
    
    461 461
             break;
    
    ... ... @@ -1936,7 +1936,7 @@ delete_threads_and_gc:
    1936 1936
                 // it will get re-enabled if we run any threads after the GC.
    
    1937 1937
                 setRecentActivity(ACTIVITY_DONE_GC);
    
    1938 1938
     #if !defined(PROFILING)
    
    1939
    -            stopTimer();
    
    1939
    +            pauseTimer();
    
    1940 1940
     #endif
    
    1941 1941
                 break;
    
    1942 1942
             }
    
    ... ... @@ -2232,8 +2232,8 @@ forkProcess(HsStablePtr *entry
    2232 2232
                 generations[g].threads = END_TSO_QUEUE;
    
    2233 2233
             }
    
    2234 2234
     
    
    2235
    -        // On Unix, all timers are reset in the child, so we need to start
    
    2236
    -        // the timer again.
    
    2235
    +        // The timer thread is not present in the child process, so we need
    
    2236
    +        // to initialise the timer again.
    
    2237 2237
             initTimer();
    
    2238 2238
     
    
    2239 2239
             // TODO: need to trace various other things in the child
    
    ... ... @@ -2244,7 +2244,7 @@ forkProcess(HsStablePtr *entry
    2244 2244
     
    
    2245 2245
             // start timer after the IOManager is initialized
    
    2246 2246
             // (the idle GC may wake up the IOManager)
    
    2247
    -        startTimer();
    
    2247
    +        unpauseTimer();
    
    2248 2248
     
    
    2249 2249
             // Install toplevel exception handlers, so interruption
    
    2250 2250
             // signal will be sent to the main thread.
    

  • rts/Ticker.h
    ... ... @@ -12,9 +12,44 @@
    12 12
     
    
    13 13
     typedef void (*TickProc)(int);
    
    14 14
     
    
    15
    -void initTicker  (Time interval, TickProc handle_tick);
    
    16
    -void startTicker (void);
    
    17
    -void stopTicker  (void);
    
    18
    -void exitTicker  (bool wait);
    
    15
    +/* The ticker is initialised in a paused state. Use unpauseTicker to start. */
    
    16
    +void initTicker(Time interval, TickProc handle_tick);
    
    17
    +
    
    18
    +/* Stop and terminate the ticker. It does not need to be stopped first.
    
    19
    + * The exitTicker action is *synchronous*. When it returns the caller is
    
    20
    + * guaranteed that the tick action is blocked.
    
    21
    + */
    
    22
    +void exitTicker(void);
    
    23
    +
    
    24
    +/* Pause and unpause (resume) the ticker.
    
    25
    + *
    
    26
    + * The pauseTicker and unpauseTicker actions are *asynchronous*. After calling
    
    27
    + * pauseTicker, the ticker will pause eventually, but there may be another tick
    
    28
    + * action before it does pause (and theoretically there could be several but
    
    29
    + * in practice this is unlikely). Similarly, after calling unpauseTicker the
    
    30
    + * ticker will start up again eventually, but there is an unspecified delay
    
    31
    + * between the unpause and the next tick action (but in practice it is short).
    
    32
    + *
    
    33
    + * This should be used for the purpose of *efficiency*: to avoid unnecessary
    
    34
    + * OS thread wakeups caused by the ticker.
    
    35
    + *
    
    36
    + * These should *not* be used for the purpose of *concurrency safety*: to
    
    37
    + * prevent the tick action from running concurrently with some other critical
    
    38
    + * section. The synchronous case is not provided because it is not currently
    
    39
    + * needed (and proper locking is often a better solution anyway).
    
    40
    + *
    
    41
    + * The pairing of unpauseTicker and the handle_tick action form a
    
    42
    + * synchonises-with relation: values written before unpauseTicker can be
    
    43
    + * read from the resulting handle_tick action.
    
    44
    + *
    
    45
    + * It *is* safe to call these functions from within the tick handler itself.
    
    46
    + *
    
    47
    + * It is safe to use these functions concurrently from multiple threads, but
    
    48
    + * note that they *are* idempotent. This means it is not appropriate to use
    
    49
    + * paired pause/unpause calls concurrently. They can be used by threads based
    
    50
    + * on consistent use of some shared state or observation.
    
    51
    + */
    
    52
    +void pauseTicker(void);
    
    53
    +void unpauseTicker(void);
    
    19 54
     
    
    20 55
     #include "EndPrivate.h"

  • rts/Timer.c
    ... ... @@ -28,15 +28,6 @@
    28 28
     #include "RtsSignals.h"
    
    29 29
     #include "rts/EventLogWriter.h"
    
    30 30
     
    
    31
    -// This global counter is used to allow multiple threads to stop the
    
    32
    -// timer temporarily with a stopTimer()/startTimer() pair.  If
    
    33
    -//      timer_enabled  == 0          timer is enabled
    
    34
    -//      timer_disabled == N, N > 0   timer is disabled by N threads
    
    35
    -// When timer_enabled makes a transition to 0, we enable the timer,
    
    36
    -// and when it makes a transition to non-0 we disable it.
    
    37
    -
    
    38
    -static StgWord timer_disabled;
    
    39
    -
    
    40 31
     /* ticks left before next pre-emptive context switch */
    
    41 32
     static int ticks_to_ctxt_switch = 0;
    
    42 33
     
    
    ... ... @@ -107,9 +98,9 @@ static
    107 98
     void
    
    108 99
     handle_tick(int unused STG_UNUSED)
    
    109 100
     {
    
    110
    -  handleProfTick();
    
    111
    -  if (RtsFlags.ConcFlags.ctxtSwitchTicks > 0
    
    112
    -      && SEQ_CST_LOAD_ALWAYS(&timer_disabled) == 0)
    
    101
    +  handleProfTick(); // Bad or worse: see issue #27250.
    
    102
    +
    
    103
    +  if (RtsFlags.ConcFlags.ctxtSwitchTicks > 0)
    
    113 104
       {
    
    114 105
           ticks_to_ctxt_switch--;
    
    115 106
           if (ticks_to_ctxt_switch <= 0) {
    
    ... ... @@ -123,7 +114,7 @@ handle_tick(int unused STG_UNUSED)
    123 114
           ticks_to_eventlog_flush--;
    
    124 115
           if (ticks_to_eventlog_flush <= 0) {
    
    125 116
               ticks_to_eventlog_flush = RtsFlags.TraceFlags.eventlogFlushTicks;
    
    126
    -          flushEventLog(NULL);
    
    117
    +          flushEventLog(NULL);  // Bad or worse: see issue #27250.
    
    127 118
           }
    
    128 119
       }
    
    129 120
     #endif
    
    ... ... @@ -148,7 +139,7 @@ handle_tick(int unused STG_UNUSED)
    148 139
                                          RtsFlags.MiscFlags.tickInterval;
    
    149 140
     #if defined(THREADED_RTS)
    
    150 141
                   wakeUpRts();
    
    151
    -              // The scheduler will call stopTimer() when it has done
    
    142
    +              // The scheduler will call pauseTimer() when it has done
    
    152 143
                   // the GC.
    
    153 144
     #endif
    
    154 145
               } else {
    
    ... ... @@ -160,10 +151,10 @@ handle_tick(int unused STG_UNUSED)
    160 151
     #if defined(PROFILING)
    
    161 152
                   if (!(RtsFlags.ProfFlags.doHeapProfile
    
    162 153
                         || RtsFlags.CcFlags.doCostCentres)) {
    
    163
    -                  stopTimer();
    
    154
    +                  pauseTimer();
    
    164 155
                   }
    
    165 156
     #else
    
    166
    -              stopTimer();
    
    157
    +              pauseTimer();
    
    167 158
     #endif
    
    168 159
               }
    
    169 160
           } else {
    
    ... ... @@ -176,48 +167,49 @@ handle_tick(int unused STG_UNUSED)
    176 167
       }
    
    177 168
     }
    
    178 169
     
    
    179
    -void
    
    180
    -initTimer(void)
    
    170
    +void initTimer(void)
    
    181 171
     {
    
    182 172
     #if defined(HAVE_PREEMPTION)
    
    183 173
         initProfTimer();
    
    184 174
         if (RtsFlags.MiscFlags.tickInterval != 0) {
    
    185 175
             initTicker(RtsFlags.MiscFlags.tickInterval, handle_tick);
    
    186 176
         }
    
    187
    -    SEQ_CST_STORE_ALWAYS(&timer_disabled, 1);
    
    188 177
     #endif
    
    189 178
     }
    
    190 179
     
    
    191
    -void
    
    192
    -startTimer(void)
    
    180
    +/* Deprecated exported functions. Now no-ops.
    
    181
    + * Historically they were used by the process and unix libraries to disable
    
    182
    + * the signal-based interval timer, since otherwise the timer signal would
    
    183
    + * keep going off in the child process and confusing everything. The interval
    
    184
    + * timer no longer uses signals, so there is no need any more for libraries to
    
    185
    + * disable the timer. Also, the timer internal API has changed.
    
    186
    + */
    
    187
    +void stopTimer(void)  { /* no-op */ }
    
    188
    +void startTimer(void) { /* no-op */ }
    
    189
    +
    
    190
    +void pauseTimer(void)
    
    193 191
     {
    
    194 192
     #if defined(HAVE_PREEMPTION)
    
    195
    -    if (SEQ_CST_SUB_ALWAYS(&timer_disabled, 1) == 0) {
    
    196
    -        if (RtsFlags.MiscFlags.tickInterval != 0) {
    
    197
    -            startTicker();
    
    198
    -        }
    
    193
    +    if (RtsFlags.MiscFlags.tickInterval != 0) {
    
    194
    +        pauseTicker();
    
    199 195
         }
    
    200 196
     #endif
    
    201 197
     }
    
    202 198
     
    
    203
    -void
    
    204
    -stopTimer(void)
    
    199
    +void unpauseTimer(void)
    
    205 200
     {
    
    206 201
     #if defined(HAVE_PREEMPTION)
    
    207
    -    if (SEQ_CST_ADD_ALWAYS(&timer_disabled, 1) == 1) {
    
    208
    -        if (RtsFlags.MiscFlags.tickInterval != 0) {
    
    209
    -            stopTicker();
    
    210
    -        }
    
    202
    +    if (RtsFlags.MiscFlags.tickInterval != 0) {
    
    203
    +        unpauseTicker();
    
    211 204
         }
    
    212 205
     #endif
    
    213 206
     }
    
    214 207
     
    
    215
    -void
    
    216
    -exitTimer (bool wait)
    
    208
    +void exitTimer (void)
    
    217 209
     {
    
    218 210
     #if defined(HAVE_PREEMPTION)
    
    219 211
         if (RtsFlags.MiscFlags.tickInterval != 0) {
    
    220
    -        exitTicker(wait);
    
    212
    +        exitTicker();
    
    221 213
         }
    
    222 214
     #endif
    
    223 215
     }

  • rts/Timer.h
    ... ... @@ -8,5 +8,12 @@
    8 8
     
    
    9 9
     #pragma once
    
    10 10
     
    
    11
    -RTS_PRIVATE void initTimer (void);
    
    12
    -RTS_PRIVATE void exitTimer (bool wait);
    11
    +#include "BeginPrivate.h"
    
    12
    +
    
    13
    +void initTimer(void);
    
    14
    +void exitTimer(void);
    
    15
    +
    
    16
    +void pauseTimer(void);
    
    17
    +void unpauseTimer(void);
    
    18
    +
    
    19
    +#include "EndPrivate.h"

  • rts/include/rts/Timer.h
    ... ... @@ -13,6 +13,6 @@
    13 13
     
    
    14 14
     #pragma once
    
    15 15
     
    
    16
    -void startTimer (void);
    
    17
    -void stopTimer  (void);
    
    16
    +void startTimer (void);    // Deprecated: see issue #27073
    
    17
    +void stopTimer (void);     // Deprecated: see issue #27073
    
    18 18
     int rtsTimerSignal (void); // Deprecated: see issue #27073

  • rts/posix/Ticker.c
    ... ... @@ -103,120 +103,112 @@
    103 103
     #include <unistd.h>
    
    104 104
     #include <fcntl.h>
    
    105 105
     
    
    106
    -static Time itimer_interval = DEFAULT_TICK_INTERVAL;
    
    107 106
     
    
    108
    -// Should we be firing ticks?
    
    109
    -// Writers to this must hold the mutex below.
    
    110
    -static bool stopped = false;
    
    107
    +// Forward declarations of local types and helper functions to hide the
    
    108
    +// difference between ppoll() and select()
    
    109
    +#if defined(HAVE_DECL_PPOLL) && HAVE_DECL_PPOLL == 1
    
    110
    +typedef struct timespec timeout;  // for ppoll()
    
    111
    +typedef struct { struct pollfd pollfds[1]; } fdset;
    
    112
    +#else
    
    113
    +typedef struct timeval timeout;   // for select()
    
    114
    +typedef struct { int fd; fd_set selectfds; } fdset; // need to stash fd
    
    115
    +#endif
    
    116
    +static void poll_init_timeout(timeout *tv, Time t);
    
    117
    +static void poll_init_fdset(fdset *fds, int fd); // single fd only
    
    118
    +// poll_*_timeout returns >0 if fd ready, ==0 if timeout, <0 if error
    
    119
    +static int poll_no_timeout(fdset *fdset);
    
    120
    +static int poll_with_timeout(fdset *fdset, timeout *t);
    
    121
    +
    
    122
    +
    
    123
    +static Time ticker_interval = DEFAULT_TICK_INTERVAL;
    
    111 124
     
    
    112
    -// should the ticker thread exit?
    
    113
    -// This can be set without holding the mutex.
    
    114
    -static bool exited = true;
    
    125
    +// Atomic variable used by client threads to communicate that they want the
    
    126
    +// ticker thread to pause. This communication is one-way, with no
    
    127
    +// acknowledgement.
    
    128
    +static bool pause_request;
    
    115 129
     
    
    116
    -// Signaled when we want to (re)start the timer
    
    117
    -static Condition start_cond;
    
    118
    -static Mutex mutex;
    
    119
    -static OSThreadId thread;
    
    130
    +// Atomic variable used by other threads to communicate that they want the
    
    131
    +// ticker thread to exit.
    
    132
    +static bool exit_request;
    
    120 133
     
    
    121
    -// fds for interrupting the ticker
    
    122
    -static int interruptfd_r = -1, interruptfd_w = -1;
    
    134
    +// Used to wait for the ticker thread to terminate after asking it to exit.
    
    135
    +static OSThreadId ticker_thread_id;
    
    123 136
     
    
    124
    -static void *itimer_thread_func(void *_handle_tick)
    
    137
    +// Fds used with sendFdWakeup to notify the ticker thread that any of the
    
    138
    +// *_request variables above have been set.
    
    139
    +static int notifyfd_r = -1, notifyfd_w = -1;
    
    140
    +
    
    141
    +static void *ticker_thread_func(void *_handle_tick)
    
    125 142
     {
    
    126 143
         TickProc handle_tick = _handle_tick;
    
    127 144
     
    
    128
    -#if defined(HAVE_DECL_PPOLL) && HAVE_DECL_PPOLL == 1
    
    129
    -    struct pollfd pollfds[1];
    
    145
    +    // Thread-local view of our state. We compare these with the corresponding
    
    146
    +    // atomic shared variables used to request state changes.
    
    147
    +    bool paused  = true;  // updated from atomic shared var pause_request
    
    148
    +    bool exit    = false; // updated from atomic shared var exit_request
    
    149
    +    // Note that we start paused.
    
    130 150
     
    
    131
    -    pollfds[0].fd = interruptfd_r;
    
    132
    -    pollfds[0].events = POLLIN;
    
    151
    +    timeout timeout;
    
    152
    +    fdset fdset;
    
    153
    +    poll_init_timeout(&timeout, ticker_interval);
    
    154
    +    poll_init_fdset(&fdset, notifyfd_r);
    
    133 155
     
    
    134
    -    struct timespec ts = { .tv_sec  = TimeToSeconds(itimer_interval)
    
    135
    -                         , .tv_nsec = TimeToNS(itimer_interval) % 1000000000
    
    136
    -                         };
    
    137
    -#else
    
    138
    -    fd_set selectfds;
    
    139
    -    FD_ZERO(&selectfds);
    
    140
    -    FD_SET(interruptfd_r, &selectfds);
    
    141
    -
    
    142
    -    struct timeval tv = { .tv_sec  = TimeToSeconds(itimer_interval)
    
    143
    -                                     /* convert remainder time in nanoseconds
    
    144
    -                                        to microseconds, rounding up: */
    
    145
    -                        , .tv_usec = ((TimeToNS(itimer_interval) % 1000000000)
    
    146
    -                                     + 999) / 1000
    
    147
    -                        };
    
    148
    -#endif
    
    156
    +    while (!exit) {
    
    149 157
     
    
    150
    -    // Relaxed is sufficient: If we don't see that exited was set in one iteration we will
    
    151
    -    // see it next time.
    
    152
    -    while (!RELAXED_LOAD_ALWAYS(&exited)) {
    
    153
    -
    
    154
    -#if defined(HAVE_DECL_PPOLL) && HAVE_DECL_PPOLL == 1
    
    155
    -        int nfds   = 1;
    
    156
    -        int nready = ppoll(pollfds, nfds, &ts, NULL);
    
    157
    -#else
    
    158
    -        struct timeval tv_tmp = tv; // copy since select may change this value.
    
    159
    -        int nfds   = interruptfd_r+1;
    
    160
    -        int nready = select(nfds, &selectfds, NULL, NULL, &tv_tmp);
    
    161
    -#endif
    
    162
    -        // In either case (ppoll or select), the result nready is the number
    
    163
    -        // of fds that are ready.
    
    164
    -        if (RTS_LIKELY(nready == 0)) {
    
    165
    -            // Timer expired, not interrupted, continue.
    
    166
    -        } else if (nready > 0) {
    
    167
    -            // We only monitor one fd (the interruptfd_r), so we know
    
    168
    -            // it is that fd that is ready without any further checks.
    
    169
    -            collectFdWakeup(interruptfd_r);
    
    170
    -            // No further action needed, continue on to handling the final tick
    
    171
    -            // and then stop.
    
    172
    -
    
    173
    -            // Note that we rely on sendFdWakeup and select/poll to provide the
    
    174
    -            // happens-before relation. So if 'exited' was set before calling
    
    175
    -            // sendFdWakeup, then we should be able to reliably read it after.
    
    176
    -            // And thus reading 'exited' in the while loop guard is ok.
    
    158
    +        int notify;
    
    159
    +        if (paused) {
    
    160
    +            notify = poll_no_timeout(&fdset);
    
    177 161
             } else {
    
    178
    -            // While the RTS attempts to mask signals, some foreign libraries
    
    179
    -            // that rely on signal delivery may unmask them. Consequently we
    
    180
    -            // may see EINTR. See #24610.
    
    181
    -            if (errno != EINTR) {
    
    182
    -                sysErrorBelch("Ticker: poll failed: %s", strerror(errno));
    
    183
    -            }
    
    162
    +            notify = poll_with_timeout(&fdset, &timeout);
    
    184 163
             }
    
    185 164
     
    
    186
    -        // first try a cheap test
    
    187
    -        if (RELAXED_LOAD_ALWAYS(&stopped)) {
    
    188
    -            OS_ACQUIRE_LOCK(&mutex);
    
    189
    -            // should we really stop?
    
    190
    -            if (stopped) {
    
    191
    -                waitCondition(&start_cond, &mutex);
    
    192
    -            }
    
    193
    -            OS_RELEASE_LOCK(&mutex);
    
    194
    -        } else {
    
    165
    +        if (RTS_LIKELY(notify == 0)) {
    
    166
    +            // The time expired, no state change notification.
    
    195 167
                 handle_tick(0);
    
    168
    +
    
    169
    +        } else if (notify > 0) {
    
    170
    +            // State change notification, check the request variables.
    
    171
    +
    
    172
    +            // We rely on sendFdWakeup and select/poll to provide the
    
    173
    +            // happens-before relation. So if the request variables are set
    
    174
    +            // before calling sendFdWakeup, then we should be able to reliably
    
    175
    +            // read them here afterwards.
    
    176
    +            collectFdWakeup(notifyfd_r);
    
    177
    +
    
    178
    +            paused = ACQUIRE_LOAD_ALWAYS(&pause_request);
    
    179
    +            exit   = RELAXED_LOAD_ALWAYS(&exit_request);
    
    180
    +        } else if (errno != EINTR) {
    
    181
    +            // While the RTS attempts to mask signals, some foreign libraries
    
    182
    +            // that rely on signal delivery may unmask them. Consequently we
    
    183
    +            // may see EINTR. See #24610.
    
    184
    +            sysErrorBelch("Ticker: poll failed: %s", strerror(errno));
    
    196 185
             }
    
    197 186
         }
    
    198 187
     
    
    199 188
         return NULL;
    
    200 189
     }
    
    201 190
     
    
    191
    +/* Initialise the ticker on startup or re-initialise the ticker after a fork().
    
    192
    + * In the fork case, the thread will not be present, but fds are inherited.
    
    193
    + *
    
    194
    + * The ticker is started in the paused state. Use unpauseTicker to continue.
    
    195
    + */
    
    202 196
     void
    
    203 197
     initTicker (Time interval, TickProc handle_tick)
    
    204 198
     {
    
    205
    -    itimer_interval = interval;
    
    206
    -    stopped = true;
    
    207
    -    exited = false;
    
    199
    +    ticker_interval     = interval;
    
    200
    +    pause_request       = true;
    
    201
    +    exit_request        = false;
    
    202
    +
    
    208 203
     #if defined(HAVE_SIGNAL_H)
    
    209 204
         sigset_t mask, omask;
    
    210 205
         int sigret;
    
    211 206
     #endif
    
    212 207
         int ret;
    
    213 208
     
    
    214
    -    initCondition(&start_cond);
    
    215
    -    initMutex(&mutex);
    
    216
    -
    
    217 209
         /* Open the interrupt fd synchronously.
    
    218 210
          *
    
    219
    -     * We used to do it in itimer_thread_func (i.e. in the timer thread) but it
    
    211
    +     * We used to do it in ticker_thread_func (i.e. in the timer thread) but it
    
    220 212
          * meant that some user code could run before it and get confused by the
    
    221 213
          * allocation of the timerfd.
    
    222 214
          *
    
    ... ... @@ -226,11 +218,11 @@ initTicker (Time interval, TickProc handle_tick)
    226 218
          * descriptor closed by the first call! (see #20618)
    
    227 219
          */
    
    228 220
     
    
    229
    -    if (interruptfd_r != -1) {
    
    221
    +    if (notifyfd_r != -1) {
    
    230 222
             // don't leak the old file descriptors after a fork (#25280)
    
    231
    -        closeFdWakeup(interruptfd_r, interruptfd_w);
    
    223
    +        closeFdWakeup(notifyfd_r, notifyfd_w);
    
    232 224
         }
    
    233
    -    newFdWakeup(&interruptfd_r, &interruptfd_w);
    
    225
    +    newFdWakeup(&notifyfd_r, &notifyfd_w);
    
    234 226
     
    
    235 227
         /*
    
    236 228
          * Create the thread with all blockable signals blocked, leaving signal
    
    ... ... @@ -242,7 +234,7 @@ initTicker (Time interval, TickProc handle_tick)
    242 234
         sigfillset(&mask);
    
    243 235
         sigret = pthread_sigmask(SIG_SETMASK, &mask, &omask);
    
    244 236
     #endif
    
    245
    -    ret = createAttachedOSThread(&thread, "ghc_ticker", itimer_thread_func, (void*)handle_tick);
    
    237
    +    ret = createAttachedOSThread(&ticker_thread_id, "ghc_ticker", ticker_thread_func, (void*)handle_tick);
    
    246 238
     #if defined(HAVE_SIGNAL_H)
    
    247 239
         if (sigret == 0)
    
    248 240
             pthread_sigmask(SIG_SETMASK, &omask, NULL);
    
    ... ... @@ -253,47 +245,112 @@ initTicker (Time interval, TickProc handle_tick)
    253 245
         }
    
    254 246
     }
    
    255 247
     
    
    256
    -void
    
    257
    -startTicker(void)
    
    248
    +/* Asynchronous. Idempotent. */
    
    249
    +void unpauseTicker(void)
    
    258 250
     {
    
    259
    -    OS_ACQUIRE_LOCK(&mutex);
    
    260
    -    RELAXED_STORE(&stopped, false);
    
    261
    -    signalCondition(&start_cond);
    
    262
    -    OS_RELEASE_LOCK(&mutex);
    
    251
    +    RELEASE_STORE_ALWAYS(&pause_request, false);
    
    252
    +    sendFdWakeup(notifyfd_w);
    
    263 253
     }
    
    264 254
     
    
    265
    -/* There may be at most one additional tick fired after a call to this */
    
    266
    -void
    
    267
    -stopTicker(void)
    
    255
    +/* Asynchronous. Idempotent.
    
    256
    + * There may be at additional ticks fired after a call to this, but it will
    
    257
    + * usually stop quickly.
    
    258
    + */
    
    259
    +void pauseTicker(void)
    
    268 260
     {
    
    269
    -    OS_ACQUIRE_LOCK(&mutex);
    
    270
    -    RELAXED_STORE(&stopped, true);
    
    271
    -    OS_RELEASE_LOCK(&mutex);
    
    261
    +    RELEASE_STORE_ALWAYS(&pause_request, true);
    
    262
    +    sendFdWakeup(notifyfd_w);
    
    272 263
     }
    
    273 264
     
    
    274
    -/* There may be at most one additional tick fired after a call to this */
    
    275
    -void
    
    276
    -exitTicker (bool wait)
    
    265
    +/* Synchronous. Not idempotent.
    
    266
    + * The ticker is guaranteed stopped after this.
    
    267
    + */
    
    268
    +void exitTicker(void)
    
    277 269
     {
    
    278
    -    ASSERT(!SEQ_CST_LOAD(&exited));
    
    279
    -    SEQ_CST_STORE(&exited, true);
    
    280
    -    // ensure that ticker wakes up if stopped
    
    281
    -    startTicker();
    
    282
    -    sendFdWakeup(interruptfd_w);
    
    283
    -
    
    284
    -    // wait for ticker to terminate if necessary
    
    285
    -    if (wait) {
    
    286
    -        if (pthread_join(thread, NULL)) {
    
    287
    -            sysErrorBelch("Ticker: Failed to join: %s", strerror(errno));
    
    288
    -        }
    
    289
    -        closeFdWakeup(interruptfd_r, interruptfd_w);
    
    290
    -        closeMutex(&mutex);
    
    291
    -        closeCondition(&start_cond);
    
    292
    -    } else {
    
    293
    -        pthread_detach(thread);
    
    270
    +    ASSERT(!RELAXED_LOAD_ALWAYS(&exit_request));
    
    271
    +    RELEASE_STORE_ALWAYS(&exit_request, true);
    
    272
    +    sendFdWakeup(notifyfd_w);
    
    273
    +
    
    274
    +    // wait for ticker thread to terminate
    
    275
    +    if (pthread_join(ticker_thread_id, NULL)) {
    
    276
    +        sysErrorBelch("Ticker: Failed to join: %s", strerror(errno));
    
    294 277
         }
    
    278
    +    closeFdWakeup(notifyfd_r, notifyfd_w);
    
    279
    +}
    
    280
    +
    
    281
    +/* Implementation of the local helpers, to hide the difference between ppoll()
    
    282
    + * and select().
    
    283
    + */
    
    284
    +#if defined(HAVE_DECL_PPOLL) && HAVE_DECL_PPOLL == 1
    
    285
    +static void poll_init_timeout(timeout *tv, Time t)
    
    286
    +{
    
    287
    +    tv->tv_sec  = TimeToSeconds(t);
    
    288
    +    tv->tv_nsec = TimeToNS(t) % 1000000000;
    
    289
    +}
    
    290
    +
    
    291
    +static void poll_init_fdset(fdset *fds, int fd)
    
    292
    +{
    
    293
    +    fds->pollfds[0].fd = fd;
    
    294
    +    fds->pollfds[0].events = POLLIN;
    
    295
    +}
    
    296
    +
    
    297
    +static int poll_no_timeout(fdset *fds)
    
    298
    +{
    
    299
    +    int nfds = 1;
    
    300
    +    return ppoll(fds->pollfds, nfds, NULL, NULL);
    
    301
    +}
    
    302
    +
    
    303
    +static int poll_with_timeout(fdset *fds, timeout *ts)
    
    304
    +{
    
    305
    +    int nfds = 1;
    
    306
    +    return ppoll(fds->pollfds, nfds, ts, NULL);
    
    307
    +}
    
    308
    +
    
    309
    +#else // select()
    
    310
    +
    
    311
    +static void poll_init_timeout(timeout *tv, Time t)
    
    312
    +{
    
    313
    +    tv->tv_sec  = TimeToSeconds(t);
    
    314
    +    /* convert remainder time in nanoseconds to microseconds, rounding up: */
    
    315
    +    tv->tv_usec = ((TimeToNS(t) % 1000000000) + 999) / 1000;
    
    316
    +}
    
    317
    +
    
    318
    +static void poll_init_fdset(fdset *fds, int fd)
    
    319
    +{
    
    320
    +    /* select() modifies the fd_set: it uses the same fd_set for reporting as
    
    321
    +     * for input. Thus we must rebuild it every time. We can optimise this
    
    322
    +     * rebuilding somewhat however if we rely on select() not modifying the
    
    323
    +     * bits that we didn't ask it to look at. So we can zero the fd_set just
    
    324
    +     * once, and then only reset the single bit for the single fd, before each
    
    325
    +     * call to selct().
    
    326
    +     */
    
    327
    +    fds->fd = fd;
    
    328
    +    FD_ZERO(&fds->selectfds);
    
    329
    +}
    
    330
    +
    
    331
    +static int poll_no_timeout(fdset *fds)
    
    332
    +{
    
    333
    +    /* select() modifies the fd_set so we must set it every time, but we rely
    
    334
    +     * on it not touching other bits to avoid having to FD_ZERO it every time
    
    335
    +     */
    
    336
    +    FD_SET(fds->fd, &fds->selectfds);
    
    337
    +    int nfds = fds->fd+1;
    
    338
    +    return select(nfds, &fds->selectfds, NULL, NULL, NULL);
    
    339
    +}
    
    340
    +
    
    341
    +static int poll_with_timeout(fdset *fds, timeout *tv)
    
    342
    +{
    
    343
    +    struct timeval tv_tmp = *tv; // copy since select may change this value.
    
    344
    +    /* select() modifies the fd_set so we must set it every time, but we rely
    
    345
    +     * on it not touching other bits to avoid having to FD_ZERO it every time
    
    346
    +     */
    
    347
    +    FD_SET(fds->fd, &fds->selectfds);
    
    348
    +    int nfds = fds->fd+1;
    
    349
    +    return select(nfds, &fds->selectfds, NULL, NULL, &tv_tmp);
    
    295 350
     }
    
    351
    +#endif
    
    296 352
     
    
    353
    +/* This is obsolete, but is used in the unix package for now */
    
    297 354
     int
    
    298 355
     rtsTimerSignal(void)
    
    299 356
     {
    

  • rts/win32/Ticker.c
    ... ... @@ -11,7 +11,11 @@
    11 11
     
    
    12 12
     static TickProc tick_proc = NULL;
    
    13 13
     static HANDLE timer_queue = NULL;
    
    14
    +
    
    15
    +static Mutex lock; // To protect the timer and paused var below
    
    14 16
     static HANDLE timer       = NULL;
    
    17
    +static bool paused;
    
    18
    +
    
    15 19
     static Time tick_interval = 0;
    
    16 20
     
    
    17 21
     static VOID CALLBACK tick_callback(
    
    ... ... @@ -36,12 +40,19 @@ static VOID CALLBACK tick_callback(
    36 40
     // This seems to be the case starting at some point during the
    
    37 41
     // Windows 7 lifetime and any newer versions of windows.
    
    38 42
     
    
    43
    +// Forward decls
    
    44
    +static void startTicker(void);
    
    45
    +static void stopTicker(bool synchronous);
    
    46
    +
    
    39 47
     void
    
    40 48
     initTicker (Time interval, TickProc handle_tick)
    
    41 49
     {
    
    50
    +    ASSERT(timer_queue == NULL);
    
    42 51
         tick_interval = interval;
    
    43 52
         tick_proc = handle_tick;
    
    44 53
     
    
    54
    +    OS_INIT_LOCK(&lock);
    
    55
    +    paused = true; // starts paused
    
    45 56
         timer_queue = CreateTimerQueue();
    
    46 57
         if (timer_queue == NULL) {
    
    47 58
             sysErrorBelch("CreateTimerQueue");
    
    ... ... @@ -49,39 +60,81 @@ initTicker (Time interval, TickProc handle_tick)
    49 60
         }
    
    50 61
     }
    
    51 62
     
    
    63
    +// Asynchronous. Idempotent.
    
    52 64
     void
    
    53
    -startTicker(void)
    
    65
    +unpauseTicker(void)
    
    54 66
     {
    
    55
    -    BOOL r;
    
    56
    -
    
    57
    -    r = CreateTimerQueueTimer(&timer,
    
    58
    -                              timer_queue,
    
    59
    -                              tick_callback,
    
    60
    -                              0,
    
    61
    -                              0,
    
    62
    -                              TimeToMS(tick_interval), // ms
    
    63
    -                              WT_EXECUTEINTIMERTHREAD);
    
    64
    -    if (r == 0) {
    
    65
    -        sysErrorBelch("CreateTimerQueueTimer");
    
    66
    -        stg_exit(EXIT_FAILURE);
    
    67
    +    OS_ACQUIRE_LOCK(&lock);
    
    68
    +    if (paused) {
    
    69
    +        startTicker();
    
    67 70
         }
    
    71
    +    paused = false;
    
    72
    +    OS_RELEASE_LOCK(&lock);
    
    68 73
     }
    
    69 74
     
    
    75
    +// Asynchronous. Idempotent.
    
    70 76
     void
    
    71
    -stopTicker(void)
    
    77
    +pauseTicker(void)
    
    72 78
     {
    
    73
    -    if (timer_queue != NULL && timer != NULL) {
    
    74
    -        DeleteTimerQueueTimer(timer_queue, timer, NULL);
    
    75
    -        timer = NULL;
    
    79
    +    OS_ACQUIRE_LOCK(&lock);
    
    80
    +    if (!paused) {
    
    81
    +        /* pauseTicker is called from within the handle_tick, so stopping
    
    82
    +         * the ticker here /must/ be asynchronous or we will deadlock! */
    
    83
    +        stopTicker(false /* asynchronous */);
    
    76 84
         }
    
    85
    +    paused = true;
    
    86
    +    OS_RELEASE_LOCK(&lock);
    
    77 87
     }
    
    78 88
     
    
    79 89
     void
    
    80
    -exitTicker (bool wait)
    
    90
    +exitTicker(void)
    
    81 91
     {
    
    82
    -    stopTicker();
    
    83
    -    if (timer_queue != NULL) {
    
    84
    -        DeleteTimerQueueEx(timer_queue, wait ? INVALID_HANDLE_VALUE : NULL);
    
    85
    -        timer_queue = NULL;
    
    92
    +    ASSERT(timer_queue != NULL);
    
    93
    +
    
    94
    +    OS_ACQUIRE_LOCK(&lock);
    
    95
    +    if (!paused) {
    
    96
    +        stopTicker(true /* synchronous */);
    
    97
    +    }
    
    98
    +    OS_RELEASE_LOCK(&lock);
    
    99
    +
    
    100
    +    // From the docs for DeleteTimerQueueEx:
    
    101
    +    //   If this parameter is INVALID_HANDLE_VALUE, the function waits
    
    102
    +    //   for all callback functions to complete before returning.
    
    103
    +    // This is a belt-and-braces approach to ensuring exitTicker is synchronous,
    
    104
    +    // since stopTicker(true) is already synchronous and there's only one timer.
    
    105
    +    HANDLE completion = INVALID_HANDLE_VALUE;
    
    106
    +    DeleteTimerQueueEx(timer_queue, completion);
    
    107
    +    timer_queue = NULL;
    
    108
    +}
    
    109
    +
    
    110
    +static void startTicker(void) {
    
    111
    +    ASSERT(timer_queue != NULL && timer == NULL);
    
    112
    +    DWORD interval = TimeToMS(tick_interval); // ms
    
    113
    +    BOOL r = CreateTimerQueueTimer(&timer,
    
    114
    +                                   timer_queue,
    
    115
    +                                   tick_callback,
    
    116
    +                                   NULL,     // callback param
    
    117
    +                                   interval, // inital interval
    
    118
    +                                   interval, // recurrant interval
    
    119
    +                                   WT_EXECUTEINTIMERTHREAD);
    
    120
    +    //TODO: using WT_EXECUTEINTIMERTHREAD is fine for context switching, and
    
    121
    +    // plausibly also ok for profile sampling but is way out for eventlog
    
    122
    +    // flushing. The eventlog flush does a global synchronisation of all
    
    123
    +    // capabilities and I/O! And with eventlog providers, it calls arbitrary
    
    124
    +    // user code. This is not ok! See issue #27250.
    
    125
    +    if (r == 0) {
    
    126
    +        sysErrorBelch("CreateTimerQueueTimer");
    
    127
    +        stg_exit(EXIT_FAILURE);
    
    86 128
         }
    
    129
    +    ASSERT(timer != NULL);
    
    130
    +}
    
    131
    +
    
    132
    +static void stopTicker(bool synchronous) {
    
    133
    +    ASSERT(timer_queue != NULL && timer != NULL);
    
    134
    +    // From the docs for DeleteTimerQueueTimer:
    
    135
    +    // If this parameter is INVALID_HANDLE_VALUE, the function waits for any
    
    136
    +    // running timer callback functions to complete before returning.
    
    137
    +    HANDLE completion = synchronous ? INVALID_HANDLE_VALUE : NULL;
    
    138
    +    DeleteTimerQueueTimer(timer_queue, timer, completion);
    
    139
    +    timer = NULL;
    
    87 140
     }

  • testsuite/tests/concurrent/should_run/T27105.hs
    1
    +{-# OPTIONS_GHC -fno-omit-yields #-}
    
    2
    +
    
    3
    +import Control.Monad
    
    4
    +import Control.Monad.ST
    
    5
    +import Control.Concurrent
    
    6
    +import Control.Exception
    
    7
    +import System.Exit
    
    8
    +import System.Mem
    
    9
    +import GHC.Arr
    
    10
    +import Prelude hiding (init)
    
    11
    +
    
    12
    +-- Test thread fairness:
    
    13
    +-- run two cpu-bound threads concurrently for a second,
    
    14
    +-- each counts how many operations it can perform until signaled to stop
    
    15
    +-- expect a balance between the two with no more than a 75% imperfection.
    
    16
    +-- Yes, 75%! On the CI machines we occasionally observe extraordinary levels
    
    17
    +-- of unfairness: nearly 60% in some cases. We don't want this to become a
    
    18
    +-- fragile test that is ignored, so we use an extreme bound. This should still
    
    19
    +-- catch gross breakage.
    
    20
    +--
    
    21
    +-- This _should_ detect if the interval timer is not working, or if thread
    
    22
    +-- context switching is messed up. We can expect failure if we force a
    
    23
    +-- contex switch interval of more than half the test time, i.e. more than 0.5s
    
    24
    +--
    
    25
    +-- We run the test twice, with allocating and non-allocating worker threads.
    
    26
    +-- The -fno-omit-yields above is crucial for worker_nonalloc below, or it never
    
    27
    +-- gets interrupted and thus no context switches.
    
    28
    +
    
    29
    +main :: IO ()
    
    30
    +main = do
    
    31
    +  test worker_alloc
    
    32
    +  performMajorGC
    
    33
    +  test worker_nonalloc
    
    34
    +
    
    35
    +test :: Worker -> IO ()
    
    36
    +test worker = do
    
    37
    +  stop <- newEmptyMVar
    
    38
    +  res1 <- newEmptyMVar
    
    39
    +  res2 <- newEmptyMVar
    
    40
    +  _ <- forkIO (worker stop >>= putMVar res1)
    
    41
    +  _ <- forkIO (worker stop >>= putMVar res2)
    
    42
    +  threadDelay 300_000
    
    43
    +  -- Let them run for 300ms. The default context switch interval is 20ms.
    
    44
    +  -- This gives time for 15 context switches, so this _should_ be enough
    
    45
    +  -- to get less than 10% unfairness. And on most platforms it is enough.
    
    46
    +  -- But OSX! Oh OSX! How do I loath thee? Let me count++ the ways.
    
    47
    +  -- To avoid a fragile test, we use a 75% unfairness threshold.
    
    48
    +  putMVar stop ()
    
    49
    +  count1 <- takeMVar res1
    
    50
    +  count2 <- takeMVar res2
    
    51
    +  let balance :: Double
    
    52
    +      balance = abs ((fromIntegral count1 - fromIntegral count2)
    
    53
    +                    / fromIntegral count2)
    
    54
    +  when (balance > 0.75) $ do
    
    55
    +    putStrLn "Schedule fairness more than 75% tolerance:"
    
    56
    +    putStrLn $ "imperfection: " ++ show (balance * 100) ++ "%"
    
    57
    +    putStrLn $ "work counts:  " ++ show (count1, count2)
    
    58
    +    exitFailure
    
    59
    +
    
    60
    +type Worker = MVar () -> IO Int
    
    61
    +
    
    62
    +-- count how many iterations we can calculate until we're signaled to stop
    
    63
    +worker_template :: IO a -> (a -> IO ()) -> MVar () -> IO Int
    
    64
    +worker_template init iter stop = do
    
    65
    +    a <- init
    
    66
    +    go a 0
    
    67
    +  where
    
    68
    +    go a !count = do
    
    69
    +      ok <- tryReadMVar stop
    
    70
    +      case ok of
    
    71
    +        Just () -> return count
    
    72
    +        Nothing -> do
    
    73
    +          iter a
    
    74
    +          go a (count + 1)
    
    75
    +
    
    76
    +
    
    77
    +-- the allocating worker
    
    78
    +{-# NOINLINE worker_alloc #-}
    
    79
    +worker_alloc :: Worker
    
    80
    +worker_alloc =
    
    81
    +  worker_template
    
    82
    +    (return 18)
    
    83
    +    (\n -> evaluate (fib n) >> return ())
    
    84
    +
    
    85
    +-- by forcing this to be Integer we cause lots of allocation!
    
    86
    +fib :: Integer -> Integer
    
    87
    +fib 0 = 0
    
    88
    +fib 1 = 1
    
    89
    +fib n = fib (n-1) + fib (n-2)
    
    90
    +
    
    91
    +
    
    92
    +-- the non-allocating worker
    
    93
    +{-# NOINLINE worker_nonalloc #-}
    
    94
    +worker_nonalloc :: Worker
    
    95
    +worker_nonalloc =
    
    96
    +  worker_template
    
    97
    +    (stToIO $ newSTArray (0,50_000) 42)
    
    98
    +    (\arr -> stToIO $ arrrev arr)
    
    99
    +
    
    100
    +arrrev :: STArray s Int Int -> ST s ()
    
    101
    +arrrev arr =
    
    102
    +    let (i,j) = boundsSTArray arr
    
    103
    +     in arrrev_go arr i j
    
    104
    +
    
    105
    +{-# NOINLINE arrrev_go #-}
    
    106
    +arrrev_go :: STArray s Int Int -> Int -> Int -> ST s ()
    
    107
    +arrrev_go !_   !i !j | i >= j = return ()
    
    108
    +arrrev_go !arr !i !j = do
    
    109
    +  x <- readSTArray arr i
    
    110
    +  y <- readSTArray arr j
    
    111
    +  writeSTArray arr i y
    
    112
    +  writeSTArray arr j x
    
    113
    +  arrrev_go arr (i+1) (j-1)
    
    114
    +

  • testsuite/tests/concurrent/should_run/all.T
    ... ... @@ -325,3 +325,15 @@ test('T26341b'
    325 325
         # test uses pipe operations which are not supported by the JS/wasm backends
    
    326 326
         , when(arch('wasm32') or arch('javascript'), skip)
    
    327 327
         , compile_and_run, ['-package process'])
    
    328
    +
    
    329
    +# Scheduler (very rough) fairness
    
    330
    +test('T27105',
    
    331
    +     [when(arch('wasm32'), skip),   # same reason as T367_letnoescape
    
    332
    +      run_timeout_multiplier(0.05)], # we expect this to run for ~2s
    
    333
    +     compile_and_run, [''])
    
    334
    +test('T27105_fail',
    
    335
    +     [when(arch('wasm32'), skip),
    
    336
    +      # And we can expect it to fail if we context switch too coarsely
    
    337
    +      extra_run_opts('+RTS -C0.2 -RTS'), expect_fail,
    
    338
    +      run_timeout_multiplier(0.05)],
    
    339
    +     multimod_compile_and_run, ['T27105.hs', ''])