I have a bug against Racket Mode that was very surprising and confusing to me: #768. A situation with many instantiate-linklet: mismatch errors in color-lexers.
I believe I finally boiled it down concurrency when using read-language, as well as calling the returned get-info procedure with e.g. a color-lexer key, which loads modules.
At first I fixed (?) this using a general purpose semaphore. Then I remembered something about a module registry lock, i.e. namespace-call-with-registry-lock, which also fixes it (AFAICT).
Here's a minimal-ish example:
#lang racket/base
(define source "#lang rhombus\ndef value = 1\n")
(define (read-language-and-get-color-lexer)
(define get-info (read-language (open-input-string source)))
(get-info 'color-lexer #f))
(define (read-language-and-get-color-lexer/with-lock)
(namespace-call-with-registry-lock (current-namespace)
read-language-and-get-color-lexer))
;; Change this to see many errors, or not. Note that, after running
;; with #t, a given REPL session may produce errors even with
;; #f, until you refresh the session and get a new namespace.
(define want-to-see-many-instantiate-linklet-errors? #t) ;; <====
(for ([_ 5])
(thread (if want-to-see-many-instantiate-linklet-errors?
read-language-and-get-color-lexer
read-language-and-get-color-lexer/with-lock)))
(sleep 1)
;; Background story:
;; https://github.com/greghendershott/racket-mode/issues/768
The concurrency vulnerable spot seems to be between calling read-language and calling the returned get-info procedure value -- another thread could muck with the namespace's module registry, there?
Beside sharing the experience, I guess my questions are:
Is this as-expected -- use of read-language by multiple threads should be guarded by the module registry lock? (Or, using a distinct current-namespace for each thread.) IOW does it sound like I finally arrived at the likely solution?
If so, should I do a doc PR for read-language to point this out, as a hint (or is that considered too obvious, and/or the consumers of this documentation too few, to make it worthwhile)?
It's true that Rhombus uses dynamic-require in get-info, but Datalog, 2D Syntax, Algol 60 and Racklog all do this too. So it looks like not worrying about module registry in get-infos has become a convention.
However, the docs for dynamic-require say that a lock is needed: "Beware that concurrent dynamic-requires in namespaces that share a module registry can create race conditions; see also namespace-call-with-registry-lock."
It seems like the lock could be taken in any of several places/levels, from lowest to the highest:
dynamic-require itself
lang info providers
tools (like Racket Mode, which don't otherwise avoid the problem via another namespace strategy as does DrR IIUC)
Two thoughts:
I wonder why dynamic-require itself does not take the lock? Although doing so might be slow, it's probably not very slow in proportion to things like file I/O and expansion?
Quite a few things in Racket "just work" in the face of concurrency, and after so many years programming in Racket I would have guessed wrong that dynamic-require does, too.
Having said that, this would be no help to existing, older versions of Racket (which e.g. Racket Mode supports for as many years as it can).
The next best choice, for who should take the lock, would be tools like Racket Mode. Why I say that: There is one implementation of dynamic-require, several tools, and "in between" a dozen-ish lang info providers. With that kind of diamond shape, just practically in terms of number of things to update, if it won't be dynamic-require itself, it should be the several tools?
I do not remember for certain why the lock is not in dynamic-require. It may be that I was worried about hiding concurrency problems that span dynamic-require calls — analogous to making variable updates atomic, which technically avoids races but does not make programs work right when multiple updates need to happen together. Probably a next step here is to try adding locking to dynamic-require and see whether that causes more problems than it solves.
If the lock cannot be put in dynamic-require, or for backward compatibility, I agree that the tools that query languages are the next-best place to put a lock. The worry for me here is that I don't understand where concurrency is coming from right now. I think of read-language as not happening concurrent to other evaluation in same namespace — and maybe that's the case for DrRacket or things I've worked on, which is maybe why I have not run into the issue. Then, having assumed serialization or isolation with respect to namespaces, I may be overlooking other problems.
FWIW, the position I've taken in DrRacket is that read-language is not safe to call concurrently in multiple threads and to always have a separate namespace for each thread when I call it so I'm also wondering about the use-case where you'd try to call it from multiple threads with the same namespace. That is, it seems unlikely that the code that gets called by read-language is going to work and the namespace issue here is only the tip of the iceberg for concurrency problems.
That's a great point. In my tentative commit, I was careful to lock around both (a) the call to read-language and (b) the call to the resulting get-info proc with the 'color-lexer key to get the lexer proc. At one point I convinced myself that was necessary, although re-testing that yesterday it did not seem to be necessary (but concurrency means maybe I was just "unlucky" not to see the problem on the re-test). Anyway in theory it seems sounds to wrap both.
It's just that, with Racket Mode, it puts each "command" (from front-end Emacs to the Racket Mode back end) into its own thread. That way, long-running commands don't block shorter ones. This command/response approach is used for things the end user would think of as a "command", as well as some internals.
Using racket-hash-lang-mode, stuff related to lang info, like color lexing and indent, is handled via more such "commands". So if (say) two Emacs buffers are opened simultaneously -- very quickly, e.g. programmatically to restore them when Emacs starts up -- then there will be multiple in-flight commands, each on its own thread. And if they are for the same language, like rhombus, there's the concurrency.
Yes, this week while I worked on this, I checked out DrR of course, and saw the "insulated read language" namespace approach.
I tried that, early on. "It worked", but (a) at that point I didn't really understand why it worked, and (b) it was many more lines of code than taking the registry lock.
So tentatively I feel like the lock is a cleaner fix, and less likely to introduce new problems?
However I admit that the lock might also fail to solve other problems I don't yet know I have, which the DrR multiplicity-of-namespaces approach would prevent!
In case it helps, in my case I think it was web-server threads that
dynamic-require a module to get the evaluated (pict) results (because
they were interpreting a user program), plus or minus a GUI thread
doing the same thing.
I'm not sure how that factors into the namespaces part, but it would
give rise to concurrency over dynamic-require.
p.s. Of course when it's time to Run a user program, that happens in a fresh dedicated namespace in a fresh dedicated custodian, etc.
But prior to that, when an Emacs buffer opens on a file, and racket-hash-lang-mode is being used (as opposed to the old, classic racket-mode), and I need to call read-language for lang-supplied code for color lexing and indent, etc.? Then the Racket Mode back end allows its own, main namespace to be used by read-language and lang info providers.
I was hoping that that library might eventually move out into a place to be reused. The reason for the extra lines has to do with wrapping the functions that are being called with checking so that the IDE can always just safely call them and get back reasonable values (well, there have been bugs but that's the idea) as well as making information that might help the language design debug errors. (That is when one of the wrapped functions returns a bad value or raises an exception the library saves a message to report to the programmer while still returning a reasonable value (typically that corresponds to the behavior you get when the language didn't try to specify whatever thing it is).) Hopefully the dependencies are okay.
Actually it occurs to me that maybe I wasn't clear enough on how this can be useful and so maybe a demonstration is in order.
One thing that can (and, sigh, does) go wrong is the lexer implementation. For example, you might insert (car #f) between these two lines in the body of scribble-lexer. If you do that, then coloring a #lang scribble/base file will fail. When that happens in DrRacket, you should see a message appearing down at the bottom of the window with a prefix "#lang" and then the error message and a "reload" button. That reload button will flush out the previous call to read-language and start up a new namespace and reload the implementation, so you can remove the (car #f) and hit the button and things will go back to working.
[[ Alas, I see that there is also a bug in the insulated read language code and you also get an internal error in drracket (or at least I did). So, still more work to be done, but I think the basic principle of the design is a good one! ]]
I hope that helps explain what the point of that library is and how it might be useful for other IDE contexts. Still more work to do on my part, clearly
Thank you for explaining more of the intent behind the insulated read-language approach -- both the contract checking and the advantage of dedicated namespaces to support reloading buggy work-in-progress lang info implementations.
It sounds like this allows DrR to be resilient while being used to develop/debug/use lang info providers.
I think the bar is probably higher in this regard, for DrR, because it is the entire editing environment? Whereas the Racket Mode back end can be restarted with M-x racket-mode-start, while the Emacs front end and user buffers remain intact. IOW a not quite so elegant variation of the DrR reload button.
Probably I will:
short term: do a simple commit guarding the status quo single namespace with the module registry lock.
medium term: think about using contracts and dedicated namespaces much like DrR's insulated read-language. (Maybe share code exposed in a future drracket-tool-text-lib, which I could use conditionally (as usual I have the challenge of supporting older versions of Racket)).
Thanks again for the additional background -- that was very useful to learn!
Tangentially, I have encountered problems with shared read-language namespaces before. The problem was that Racket source can contain embedded images via the WXME format, and just reading WXME files amounts to instantiating some (2htdp) image snip classes. This means that the namespace used for reading the file needs to be the one used for running the file, for otherwise the Racket classes could not recognize each other.
Luckily, this is probably not an issue elsewhere because embedded images (and other snips like collapsible S-expressions and comment boxes) mostly appear in HtDP + DrRacket.
