Skip to content

Commit ee10cb4

Browse files
committed
py/mphal: Declare the shared wake descriptor that extmod calls.
extmod/modselect.c reaches mp_hal_wake_event_fd() and mp_hal_wake_event_drain() when it has no per-thread wake object to inject, but both were declared only in ports/unix/mphalport.h. Portable code was calling a port-private entry point that no contract described, and the two sides did not even agree on when it existed: modselect tested the descriptor capability while the port tested the wake-object one. They are now declared in py/mphal.h behind MICROPY_HAL_HAS_WAKE_EVENT_FD, which a port sets when its shared wake primitive can offer a descriptor. The capability is independent of the wake-object one rather than its negation, because a port may have neither: a wake object with no descriptor is what an event register or a task notification looks like. That combination now selects the third branch and falls back to the period-capped sweep, where before it failed to compile. Signed-off-by: Andrew Leech <andrew@alelec.net>
1 parent 1bbd1f7 commit ee10cb4

5 files changed

Lines changed: 52 additions & 22 deletions

File tree

extmod/modselect.c

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -435,8 +435,13 @@ static struct pollfd *poll_set_add_fd(poll_set_t *poll_set, int fd) {
435435
static bool poll_set_signal_wake_is_reliable(void) {
436436
#if MICROPY_HAL_WAKE_OBJ_HAS_POSIX_FD
437437
return true;
438-
#else
438+
#elif MICROPY_HAL_WAKE_EVENT_HAS_POSIX_FD
439439
return mp_sched_thread_can_run_callbacks() && mp_hal_wake_event_fd() >= 0;
440+
#else
441+
// Neither a per-thread object nor a shared descriptor to put in the set, so nothing
442+
// here can end a deadline block early and the period-capped sweep is the only correct
443+
// cadence for a signal source.
444+
return false;
440445
#endif
441446
}
442447
#endif
@@ -906,13 +911,17 @@ static mp_uint_t poll_set_poll_until_ready_or_timeout(poll_set_t *poll_set, size
906911
// way, as neither relies on this reserved slot for its own readiness.
907912
#if MICROPY_HAL_WAKE_OBJ_HAS_POSIX_FD
908913
poll_set->pollfds[0].fd = mp_hal_wake_obj_posix_fd(wake_obj);
909-
#else
914+
#elif MICROPY_HAL_WAKE_EVENT_HAS_POSIX_FD
910915
// No per-thread wake object on this port or build: the shared wake event is the
911916
// only mechanism left, safe to inject for the same entitlement
912917
// poll_set_signal_wake_is_reliable() checks above (at most one thread ever holds
913918
// it), and -1 for every other thread so its poll() always falls back to the
914919
// periodic sweep below.
915920
poll_set->pollfds[0].fd = mp_sched_thread_can_run_callbacks() ? mp_hal_wake_event_fd() : -1;
921+
#else
922+
// No wake primitive this poll set can be given, so the slot stays empty and every
923+
// signal source falls back to the periodic sweep below.
924+
poll_set->pollfds[0].fd = -1;
916925
#endif
917926
poll_set->pollfds[0].events = POLLIN;
918927
poll_set->pollfds[0].revents = 0;
@@ -995,7 +1004,7 @@ static mp_uint_t poll_set_poll_until_ready_or_timeout(poll_set_t *poll_set, size
9951004
if (poll_set->pollfds[0].revents != 0) {
9961005
#if MICROPY_HAL_WAKE_OBJ_HAS_POSIX_FD
9971006
mp_hal_wake_obj_drain(wake_obj);
998-
#else
1007+
#elif MICROPY_HAL_WAKE_EVENT_HAS_POSIX_FD
9991008
mp_hal_wake_event_drain();
10001009
#endif
10011010
}

ports/unix/mpconfigport.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,11 @@ typedef long mp_off_t;
190190
#define MICROPY_HAL_HAS_WAKE_OBJ (MICROPY_PY_THREAD)
191191
#define MICROPY_HAL_WAKE_OBJ_HAS_POSIX_FD (MICROPY_PY_THREAD)
192192

193+
// The shared wake event is opened, and so has a descriptor to offer, only on a build with
194+
// no per-thread objects; with them every waiter has its own and the shared one has no
195+
// reader (see mp_hal_wake_event_init()).
196+
#define MICROPY_HAL_WAKE_EVENT_HAS_POSIX_FD (!MICROPY_HAL_HAS_WAKE_OBJ)
197+
193198
// Bare-metal ports don't have stderr. Printing debug to stderr may give tests
194199
// which check stdout a chance to pass, etc.
195200
extern const struct _mp_print_t mp_stderr_print;

ports/unix/mphalport.h

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -145,21 +145,8 @@ void mp_hal_wake_event_wait_ms(mp_uint_t timeout_ms);
145145
// timeout. Returns 0 on timeout, or -1 with errno EINTR if cut short.
146146
int mp_hal_wake_event_wait_tv(struct timeval *tv);
147147

148-
// The descriptor the wake event can be waited on, for a caller that runs its own
149-
// poll set and wants the event in it alongside its own descriptors, or -1 when
150-
// there is none to give. Defined only where the wake event is backed by a
151-
// descriptor, so on this port and not on Windows.
152-
//
153-
// Drain it after, and only after, it has polled readable. It is
154-
// level-triggered, so it stays readable until drained, and draining before a
155-
// wait instead discards a raise and leaves that wait with nothing to end it.
156-
//
157-
// Declared only where per-thread wake objects are unavailable: with them, a poll set
158-
// injects this thread's own object instead and these have no caller.
159-
#if !MICROPY_HAL_HAS_WAKE_OBJ
160-
int mp_hal_wake_event_fd(void);
161-
void mp_hal_wake_event_drain(void);
162-
#endif
148+
// mp_hal_wake_event_fd() and mp_hal_wake_event_drain() are declared in py/mphal.h, behind
149+
// MICROPY_HAL_WAKE_EVENT_HAS_POSIX_FD, because extmod/modselect.c calls them.
163150

164151
#if MICROPY_PY_BLUETOOTH
165152
enum {

ports/unix/unix_mphal.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,10 @@ typedef uint8_t wake_event_token_t;
5252
// wake_event_fd drains it, writing wake_event_wr_fd raises it; on Linux both
5353
// are the same descriptor, and both are negative when not initialised.
5454
// Only exists where something waits on it; with per-thread wake objects every waiter has
55-
// its own and this pair has no reader (see mp_hal_wake_event_init()).
55+
// its own and this pair has no reader (see mp_hal_wake_event_init()). Keyed on the wake
56+
// mechanism, not on MICROPY_HAL_WAKE_EVENT_HAS_POSIX_FD: that macro says whether this port hands
57+
// the descriptor out to extmod, which is a different question from whether the event
58+
// exists at all.
5659
#if !MICROPY_HAL_HAS_WAKE_OBJ
5760
static int wake_event_fd = -1;
5861
static int wake_event_wr_fd = -1;
@@ -435,7 +438,7 @@ int mp_hal_wake_event_wait_tv(struct timeval *tv) {
435438
return select(0, NULL, NULL, NULL, tv);
436439
}
437440

438-
#if !MICROPY_HAL_HAS_WAKE_OBJ
441+
#if MICROPY_HAL_WAKE_EVENT_HAS_POSIX_FD
439442
// The shared event's descriptor, for a poll set to sleep on alongside its own entries.
440443
// Only where per-thread wake objects are unavailable: with them, extmod/modselect.c
441444
// injects this thread's own object instead and these have no caller.

py/mphal.h

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,12 @@ void mp_hal_wake_obj_signal_all(void);
165165
// How a waiter composes its own object with everything else it is waiting on has no
166166
// port-neutral spelling, so it is deliberately not part of the contract above. Each port
167167
// that adopts wake objects exports whatever composition primitive it has behind its own
168-
// capability macro, and core code that uses one is guarded to match. The descriptor below
169-
// is the only such primitive today.
168+
// capability macro, and core code that uses one is guarded to match. A POSIX descriptor is
169+
// the only such primitive today, for either flavour of wake primitive: the one below for a
170+
// per-thread object, and MICROPY_HAL_WAKE_EVENT_HAS_POSIX_FD further down for a port whose
171+
// wake primitive is process-wide. Both are consumed only inside
172+
// MICROPY_PY_SELECT_POSIX_OPTIMISATIONS, since a struct pollfd is the only thing that
173+
// wants them.
170174
#ifndef MICROPY_HAL_WAKE_OBJ_HAS_POSIX_FD
171175
#define MICROPY_HAL_WAKE_OBJ_HAS_POSIX_FD (0)
172176
#endif
@@ -183,6 +187,28 @@ void mp_hal_wake_obj_signal_all(void);
183187
int mp_hal_wake_obj_posix_fd(mp_hal_wake_obj_t *w);
184188
#endif
185189

190+
// A port with one process-wide wake primitive rather than per-thread objects may still be
191+
// able to hand its descriptor to a caller running its own poll()/select() set. That
192+
// primitive has exactly one legitimate consumer, whichever waiter drains it first, so a
193+
// caller must satisfy itself that this thread is the one entitled to it
194+
// (mp_sched_thread_can_run_callbacks()) before waiting on the descriptor.
195+
//
196+
// Independent of the wake-object capability above rather than its negation: a port may
197+
// have neither, and core code must then fall back to a bounded re-check instead of
198+
// blocking on something nothing can end.
199+
#ifndef MICROPY_HAL_WAKE_EVENT_HAS_POSIX_FD
200+
#define MICROPY_HAL_WAKE_EVENT_HAS_POSIX_FD (0)
201+
#endif
202+
203+
#if MICROPY_HAL_WAKE_EVENT_HAS_POSIX_FD
204+
// The shared wake primitive's descriptor, or negative when there is none to give (the HAL
205+
// has been torn down). Drain it after, and only after, it has polled readable: it is
206+
// level-triggered, so draining ahead of a wait discards the raise and leaves that wait
207+
// with nothing to end it.
208+
int mp_hal_wake_event_fd(void);
209+
void mp_hal_wake_event_drain(void);
210+
#endif
211+
186212
#if MICROPY_PY_SELECT_EVENT_SOURCE
187213

188214
// Monotonic count of mp_event_signal() calls. A waiter compares a snapshot of this

0 commit comments

Comments
 (0)