Add macros for mutex use around libc functions that need them

[khwilliamson]

This replaces #22283
This file is intended to insulate C code from thread issues, when used as directed.

This commit merely creates the file; it is not #included yet.

• This set of changes requires a perldelta entry, and I will write it before merging this

[khwilliamson]

@tonycoz wrote

Why does this generate:

#  error_foo_not suitable...

rather than

#  error foo_not suitable...

? The first may produce an error, but it might not include the text of the symbol.

The second will produce the requested message, though it might be better to quote it, which will prevent macro replacement, allowing you to avoid the need for all_the_underscores.

[khwilliamson]

The answer is that it turns out that #error is not valid as an expansion of a #define. So, I got rid of the # and the symbol should show up including 'foo'

[khwilliamson]

@tonycoz wrote:

Two issues, in general:

1. This defines a huge number of names most of which will never be used, and have a potential to conflict with names from other libraries.

I think this should add a PERL_ prefix to each lock name.

ok

2. This defines separate symbols for each function in a closely related group of functions, for example `dbm_*` and any read locale function.

The separate macros imply the need to do each of those locks, even though in some cases it doesn't make sense, eg. if I do a dbm_delete(), I really don't want to release the lock until I do dbm_error().

I think macros should be defined for groups of such functions where it makes sense, eg. a single macro for the dbm_* functions, and just document the need for LC_*_LOCK for locale reading functions etc.

I don't know what to do about this. First, for the casual reader, these all expand to no-ops except on a threaded build.

This commit doesn't have anything about how these locks are actually implemented. My proposed implementation, I believe avoids the possibility of deadlock, and doesn't introduce any new mutexes, which doing would make deadlock possible. These locks look like they have three possible mutexes, the existing ENV and LOCALE ones, plus a GENeric one. That one is mapped to either of the other two, depending on the Configuration. It turns out on systems with thread-safe locales, the LOCALE mutex is essentially unused, so is available for the generic one. Otherwise, that mutex can get used a lot, and so the ENV mutex is used instead, as we expect changes to the environment to be fairly rare. The ENV and LOCALE mutexes have co-existed for quite a few releases now without deadlock reports. Deadlock is avoided by always doing things in a particular order.

By using a common mutex for unrelated libc calls, threads block that wouldn't if there were a mutex for just the related libc calls. For example asctime needs to lock out threads that are simultaneously executing itself or ctime and nothing else.. But this implementation locks out threads executing a whole host of other libc calls, such as drand48. If there were a mutex for each possible group, you would find that there would be overlap, so that to fully protect such a group would require multiple mutexes, which greatly increases the likelihood of deadlock occurring. My belief is that the performance is going to be acceptable when the common mutex is just for a single libc call that returns quickly.

But if we use a common mutex for a series of operations, unlocking it at the end, threads using unrelated operations which use the same mutex are locked out for the duration. It would be better I think to have a new mutex for just the series. This would entail that the code using it be carefully crafted to not have deadlock potential.

And, then would there be a mutex for each type of series, like dbm and pwent'?

I do have text in the comments indicating there is an issue here. But maybe the solution is to just not generate locks for functions that are part of a series

[tonycoz]

My belief is that the performance is going to be acceptable when the common mutex is just for a single libc call that returns quickly.

The problem is that something like:

PERL_DBM_STORE_LOCK;
int status = dbm_store(db, key, value, DBM_INSERT);
PERL_DBM_STORE_UNLOCK;
if (status < 0) {
  PERL_DBM_ERROR_LOCK;
  printf("error %d\n", dbm_error(db));
  PERL_DBM_ERROR_UNLOCK;
}

is invalid, since other operations may have occurred in the gaps between locks to invalidate the stored error value.

Now the user could just lock everything they need in a block:

PERL_DBM_STORE_LOCK;
PERL_DBM_ERROR_LOCK;
int status = dbm_store(db, key, value, DBM_INSERT);
if (status < 0) {
  printf("error %d\n", dbm_error(db));
}
PERL_DBM_ERROR_UNLOCK;
PERL_DBM_STORE_UNLOCK;

but that's wasteful, and annoying.

Note that the extended span of the lock here is about guarding access to a working API, not just about preventing crashes that might occur if there were multiple concurrent dbm_store()s in progress.

[khwilliamson]

I removed the locks for functions that need to be used in conjunction with other ones as an atomic unit

[tonycoz]

fucntion name

[khwilliamson]

done

[tonycoz]

This commit merely creates the file; it is not #included yet.

I assume (hope?) it will be perl's headers including this.

In any case, I think lock_definitions.h should have a perl related name to avoid conflicts with headers from other sources.

Whether that's perl_locks,h, perl_lock_definitions.h I'm not too worried.

[khwilliamson]

I have now added commits intended to complete this pull request. Previously, only the file lock_definitions.h was included. This #includes it in perl.h and creates two different implementations for them, and adds some usage for them in POSIX.xs. Still needed are changes to perlclib, perlapi, and perldelta to refer to these.

The intent is that an XS writer or perl core writer can wrap calls to libc functions with the appropriate macros without needing to understand the nuances, and get thread-safe operation without the possibility of deadlock

One implementation is for when there are thread-safe locales. In this case, an existing mutex is essentially unused, and can be repurposed. The other implementation works, with somewhat less performance, otherwise.

The file is not intended to be included directly into XS space, but the definitions are always there to anyone who includes perl.h