Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REPL: add lock to orchestrate adding repl modes etc. #54890

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

IanButterworth
Copy link
Sponsor Member

@IanButterworth IanButterworth changed the title REPL: make repl_mutate_lock global to orchestrate adding repl modes etc. REPL: add lock to orchestrate adding repl modes etc. Jun 22, 2024
@IanButterworth IanButterworth added the stdlib:REPL Julia's REPL (Read Eval Print Loop) label Jun 22, 2024
@vtjnash
Copy link
Sponsor Member

vtjnash commented Jun 22, 2024

Taking a lock here does not help with the general unsoundness of this being written inside an __init__ method

@IanButterworth
Copy link
Sponsor Member Author

So you're saying there's no valid way to initialize the repl mode automatically when calling using Pkg?

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jun 22, 2024

Correct. Arbitrarily mutating some random global state during __init__ in the hopes that it was the correct local state to change is a bad idea

@IanButterworth
Copy link
Sponsor Member Author

IanButterworth commented Jun 22, 2024

Pkg aside, if we take a 3rd party package that adds a repl mode, what's the valid way for a user to initialize then?

startup.jl

import CustomREPLMode
CustomREPLMode.init()

and in the REPL itself

julia> import CustomREPLMode

julia> CustomREPLMode.init()

In particular, is the latter any better for knowing that the init is happening during the correct repl state?

I thought a lock was a logical fix here. Maybe there are more places in REPL that should take the lock.

cc. @KristofferC given discussion in JuliaLang/Pkg.jl#3933

@IanButterworth
Copy link
Sponsor Member Author

I do think the lock is an improvement here. Should we move forward with this, or is there some other automatic initializing method other than __init__ that you have in mind (for 1.11+) @vtjnash ?

@IanButterworth
Copy link
Sponsor Member Author

Bump

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib:REPL Julia's REPL (Read Eval Print Loop)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pkg.REPLExt is not safe to load multithreaded very unsound assumptions about REPL state injection
2 participants