sem_acquire has no reason to do a notify! #857

Merged

kilograham merged 1 commit from sem_acquire_fix into develop

2022-06-20 09:52:43 -05:00

kilograham commented

2022-06-08 13:26:11 -05:00

(Migrated from github.com)

No description provided.

Wren6991 (Migrated from github.com) reviewed

2022-06-08 13:26:11 -05:00

slimhazard commented

2022-06-10 01:07:18 -05:00

(Migrated from github.com)

The source for sem_release() has this todo comment:

// todo this should really have a blocking variant for when permits are maxed out

If there is ever a sem_release_blocking(), then the acquires will need the notifications. If the todo is ever to come true, maybe 1.3.2 is the time for it?

sem_release_blocking() would probably want to have the variations on the theme that the acquires have -- timeouts, until and try.

Or maybe delete the todo, and not have sem_release_blocking()? I personally have never needed that, and don't know of algorithms that require it. But I certainly don't know all of the ways semaphores are used. The caller of sem_release() can check its return value, FWIW.

By comparison, POSIX semaphores don't configure a maximum value the way the SDK sets max_permits (SEM_VALUE_MAX is a resource constraint, since the counter can only go so high). As a consequence, the POSIX API doesn't foresee blocking, timeout or try variants for sem_post(). On Linux, sem_post() returns an error if the count exceeds SEM_VALUE_MAX, but POSIX apparently doesn't require that.

Which tends to persuade me that sem_release_blocking() is not needed, the todo can be deleted, and this comment may have been much ado about nothing. In which case apologies, but I think I'll hit the submit button anyway, because it seems to be worth thinking through,

The source for `sem_release()` has this todo comment: ``` // todo this should really have a blocking variant for when permits are maxed out ``` If there is ever a `sem_release_blocking()`, then the acquires will need the notifications. If the todo is ever to come true, maybe 1.3.2 is the time for it? `sem_release_blocking()` would probably want to have the variations on the theme that the acquires have -- timeouts, until and try. Or maybe delete the todo, and not have `sem_release_blocking()`? I personally have never needed that, and don't know of algorithms that require it. But I certainly don't know all of the ways semaphores are used. The caller of `sem_release()` can check its return value, FWIW. By comparison, POSIX semaphores don't configure a maximum value the way the SDK sets `max_permits` (`SEM_VALUE_MAX` is a resource constraint, since the counter can only go so high). As a consequence, the POSIX API doesn't foresee blocking, timeout or try variants for `sem_post()`. On Linux, `sem_post()` returns an error if the count exceeds `SEM_VALUE_MAX`, but POSIX apparently doesn't require that. Which tends to persuade me that `sem_release_blocking()` is not needed, the todo can be deleted, and this comment may have been much ado about nothing. In which case apologies, but I think I'll hit the submit button anyway, because it seems to be worth thinking through,

kilograham commented

2022-06-12 16:43:06 -05:00

(Migrated from github.com)

The source for sem_release() has this todo comment:
// todo this should really have a blocking variant for when permits are maxed out
If there is ever a sem_release_blocking(), then the acquires will need the notifications. If the todo is ever to come true, maybe 1.3.2 is the time for it?

sem_release_blocking() would probably want to have the variations on the theme that the acquires have -- timeouts, until and try.

Or maybe delete the todo, and not have sem_release_blocking()? I personally have never needed that, and don't know of algorithms that require it. But I certainly don't know all of the ways semaphores are used. The caller of sem_release() can check its return value, FWIW.

By comparison, POSIX semaphores don't configure a maximum value the way the SDK sets max_permits (SEM_VALUE_MAX is a resource constraint, since the counter can only go so high). As a consequence, the POSIX API doesn't foresee blocking, timeout or try variants for sem_post(). On Linux, sem_post() returns an error if the count exceeds SEM_VALUE_MAX, but POSIX apparently doesn't require that.

Which tends to persuade me that sem_release_blocking() is not needed, the todo can be deleted, and this comment may have been much ado about nothing. In which case apologies, but I think I'll hit the submit button anyway, because it seems to be worth thinking through,

Yup; agreed sem_release_blocking would require the additional of new notifications/waits however these changes would remain the same (I think)

> The source for `sem_release()` has this todo comment: > > ``` > // todo this should really have a blocking variant for when permits are maxed out > ``` > > If there is ever a `sem_release_blocking()`, then the acquires will need the notifications. If the todo is ever to come true, maybe 1.3.2 is the time for it? > > `sem_release_blocking()` would probably want to have the variations on the theme that the acquires have -- timeouts, until and try. > > Or maybe delete the todo, and not have `sem_release_blocking()`? I personally have never needed that, and don't know of algorithms that require it. But I certainly don't know all of the ways semaphores are used. The caller of `sem_release()` can check its return value, FWIW. > > By comparison, POSIX semaphores don't configure a maximum value the way the SDK sets `max_permits` (`SEM_VALUE_MAX` is a resource constraint, since the counter can only go so high). As a consequence, the POSIX API doesn't foresee blocking, timeout or try variants for `sem_post()`. On Linux, `sem_post()` returns an error if the count exceeds `SEM_VALUE_MAX`, but POSIX apparently doesn't require that. > > Which tends to persuade me that `sem_release_blocking()` is not needed, the todo can be deleted, and this comment may have been much ado about nothing. In which case apologies, but I think I'll hit the submit button anyway, because it seems to be worth thinking through, Yup; agreed sem_release_blocking would require the additional of new notifications/waits however these changes would remain the same (I think)

liamfraser (Migrated from github.com) approved these changes

2022-06-20 04:17:30 -05:00