[issue42767] Review usage of atomic variables in signamodule.c

[Ammar Askar]

Ammar Askar  added the comment:

> For me, the most surprising part is Handlers[signum].tripped which is 
> declared as volatile *and* an atomic variable.

I think it's just following this part of the C spec (Some background in 
bpo-12060):

> or refers to any object with static storage duration other than by assigning 
> a value to a static storage duration variable of type volatile sig_atomic_t. 
> Furthermore, if such a call fails, the value of errno is unspecified.

https://pubs.opengroup.org/onlinepubs/007904875/functions/sigaction.html / 
https://wiki.sei.cmu.edu/confluence/display/c/SIG31-C.+Do+not+access+shared+objects+in+signal+handlers

> I'm not sure if Handlers[signum].func must be volatile neither.

I think your assessment here is right considering it's never used in the signal 
handler itself.

> Also, wakeup.fd is declared as sig_atomic_t on Unix. Could we use an atomic 
> variable instead, or is it important to use "sig_atomic_t"?

Again looking at the 
https://wiki.sei.cmu.edu/confluence/display/c/SIG31-C.+Do+not+access+shared+objects+in+signal+handlers
 recommendation it seems like using an atomic variable is fine but only if it's 
lock-free.

--
nosy: +ammar2

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42767] Review usage of atomic variables in signamodule.c

[hai shi]

Change by hai shi :


--
nosy: +shihai1991

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42767] Review usage of atomic variables in signamodule.c

[Dong-hee Na]

Change by Dong-hee Na :


--
nosy: +corona10

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42767] Review usage of atomic variables in signamodule.c

[STINNER Victor]

New submission from STINNER Victor :

In bpo-41713, I ported the _signal module to the multi-phase initialization 
API. I tried to move the signal state into a structure to cleanup the code, but 
I was scared by the following atomic variables:
---
static volatile struct {
_Py_atomic_int tripped;
PyObject *func;
} Handlers[NSIG];

#ifdef MS_WINDOWS
#define INVALID_FD ((SOCKET_T)-1)

static volatile struct {
SOCKET_T fd;
int warn_on_full_buffer;
int use_send;
} wakeup = {.fd = INVALID_FD, .warn_on_full_buffer = 1, .use_send = 0};
#else
#define INVALID_FD (-1)
static volatile struct {
#ifdef __VXWORKS__
int fd;
#else
sig_atomic_t fd;
#endif
int warn_on_full_buffer;
} wakeup = {.fd = INVALID_FD, .warn_on_full_buffer = 1};
#endif

/* Speed up sigcheck() when none tripped */
static _Py_atomic_int is_tripped;
---

For me, the most surprising part is Handlers[signum].tripped which is declared 
as volatile *and* an atomic variable.

I'm not sure if Handlers[signum].func must be volatile neither.

Also, wakeup.fd is declared as sig_atomic_t on Unix. Could we use an atomic 
variable instead, or is it important to use "sig_atomic_t"?

--

I recently added pycore_atomic_funcs.h which provides functions to access 
variables atomically. It uses atomic functions if available, or falls back on 
"volatile" otherwise. Maybe this approach would be interesting here, maybe for 
Handlers[signum].func?

--
components: Interpreter Core
messages: 383901
nosy: vstinner
priority: normal
severity: normal
status: open
title: Review usage of atomic variables in signamodule.c
versions: Python 3.10

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com