[ base ] Add atomically function

[Matthew-Mosior]

This PR adds a new function, atomically to Data.IORef (only for the chez backend).

This function atomically runs its argument according to the provided mutex.

It can for instance be used to modify the contents of an IORef ref with a function f in a safe way in a multithreaded program by using atomically lock (modifyIORef ref f) provided that other threads also rely on the same lock to modify ref.

Credit to @stefan-hoeck for providing code to test this addition in chez003.

Thanks to @gallais for the generalized atomically implementation.

[stefan-hoeck]

This cannot work. Two threads that compete for an atomic reference must be using the same mutex. Here, you create a new mutex for every new call to atomicModifyIORef which is quite pointless. So, the mutex must either be an argument to atomicModifyIORef (see my example on discord) or wrapped up in IORef itself, but that would probably slow down everything else.

[stefan-hoeck]

In order to proof atomicity, the two competing threads must be long running (hundreds of milliseconds) and both access the mutable reference a lot (thousands or better millions of accesses). Preferably, one is considerably slower than the other. Only then will you create enough contention to truly show atomicity. Again, see my example on discord.

In this example, creating and acquiring the mutex (which you currently do on every call to atomicModifyIORef) is so slow (mutexes are slow) compared to the simple update action, that you will almost always get the illusion of atomicity. Try running your version of atomicModifyIORef with the example code I gave on discord and you will see that it is not atomic at all.

[Matthew-Mosior]

Ah I see, the code I added in chez003 never actually created any true contention thus never tested the atomicity of atomicModifyIORef. I added some of the code from discord you provided for chez003, thank you for providing this, and thanks for helping me see/understand this!

[stefan-hoeck]

One more thought on this: I think in your implementation of atomicModifyIORef you are trying to make sure that the mutex version is only used on Scheme backends. I'm afraid, this won't work, because the compiler has to generate code for every possible branch in your pattern match (but maybe I'm mistaken, and this is indeed how such tests should be performed). In order to verify this, you should add a test that tries compiling and running some code with atomicModifyIORef on Node.js.

Again, there are several possibilities to approach this if the current version does not work:

• Let the decision if atomicModifyIORef should be used or not be made in client code.
• Write dummy implementations of prim__makeMutex and friends for the JavaScript backends. You could, for instance, just return 0 or undefined for prim__makeMutex on JavaScript backends. mutexAcquire and mutexRelease would then just be no-ops on those backends, since they are single-threaded anyway.

[Matthew-Mosior]

One more thought on this: I think in your implementation of atomicModifyIORef you are trying to make sure that the mutex version is only used on Scheme backends. I'm afraid, this won't work, because the compiler has to generate code for every possible branch in your pattern match (but maybe I'm mistaken, and this is indeed how such tests should be performed). In order to verify this, you should add a test that tries compiling and running some code with atomicModifyIORef on Node.js.

Again, there are several possibilities to approach this if the current version does not work:

* Let the decision if `atomicModifyIORef` should be used or not be made in client code.

* Write dummy implementations of `prim__makeMutex` and friends for the JavaScript backends. You could, for instance, just return 0 or `undefined` for `prim__makeMutex` on JavaScript backends. `mutexAcquire` and `mutexRelease` would then just be no-ops on those backends, since they are single-threaded anyway.

Thank you for the feedback and the possible solutions.

I have decided to just let the client make the decision of when to utilize atomicModifyIORef, which seems like the simplest solution currently (this should be addressed via 0fb8ed7).

Might be cool to add some dummy primitives for the javascript backend in the future though.

[Matthew-Mosior]

@stefan-hoeck @gallais

Not sure what to do about the failing checks (pretty much the same issue for all of them):

Downloading single artifact
Error: Unable to download artifact(s): Artifact not found for name: ubuntu-installed-idris2-0.7.0-chez
        Please ensure that your artifact is not expired and the artifact was uploaded using a compatible version of toolkit/upload-artifact.
        For more information, visit the GitHub Artifacts FAQ: https://github.com/actions/toolkit/blob/main/packages/artifact/docs/faq.md

[mattpolzin]

RE failing checks, it may just be that GitHub broke home-directory resolution in their paths for artifacts. I am testing this theory out now.

[EDIT] I was wrong. But I did spot a suspicious looking "include hidden files: false" setting that might be causing this problem.

[Matthew-Mosior]

RE failing checks, it may just be that GitHub broke home-directory resolution in their paths for artifacts. I am testing this theory out now.

[EDIT] I was wrong. But I did spot a suspicious looking "include hidden files: false" setting that might be causing this problem.

Thank you for looking into this!

[mattpolzin]

Testing the hidden file theory now. Very promising given the recency of actions/upload-artifact#602.

[EDIT] Looks good. Fix is up for PR.

[mattpolzin]

You're good to merge main into this branch when you get the chance and CI should work again.

[Matthew-Mosior]

You're good to merge main into this branch when you get the chance and CI should work again.

Sounds good, just merged main in!

[gallais]

Unrelated but this seems unsafe: by writing a linear value into a reference, we gain the ability to freely duplicate it!