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

Restore IRB.CurrentContext when binding.irb is nested #936

Closed
wants to merge 2 commits into from

Conversation

tompng
Copy link
Member

@tompng tompng commented Apr 27, 2024

Fix #650

$ irb
irb(main):001> 123.instance_eval{binding.irb}
irb(123):001> exit
=> nil
irb(main):002> IRB.CurrentContext.main
=> 123 # Should be `main`. `IRB.CurrentContext` is not restored

Changes

Moved preparation of conf[:MAIN_CONTEXT] to eval_input.
Every eval_input was working before because we forgot to restore MAIN_CONTEXT, so MAIN_CONTEXT was always present.

Delete conf parameter from def run which is always identical to IRB.conf, so we don't have to pass that param to eval_input.

@tompng tompng added the bug Something isn't working label Apr 27, 2024
In non-nested job controlled binding.irb in multithread, Restoring context is not enough.
Set MAIN_CONTEXT just before readmultiline will reduce the chance of MAIN_CONTEXT being wrong.
@tompng
Copy link
Member Author

tompng commented Apr 27, 2024

I added the second commit just in case to reduce unexpected MAIN_CONTEXT restored to nil in multithread which will cause IRB crash. When binding.irb is executed in job-controlled multithread, non-nested binding.irb may start and end in random order.
Even with the second commit, there is a chance MAIN_CONTEXT being nil or wrong because we don't use mutex or thread local variable yet. (Example: uncontrolled multithread binding.irb)

Example of job-controlled multiple binding.irb.

def current
  [self, IRB.CurrentContext.main]
end
def pass(thread_id = nil)
  thread_id ||= next_thread_id
  raise "No such thread #{thread_id}" unless $queues[thread_id]
  $queues[thread_id].enq :pass
  $queues[$threads.index(Thread.current)].deq
  nil
end
def next_thread_id
  thread_id = $threads.index(Thread.current)
  (1..4).map { (thread_id + _1) % $queues.size }.find { $queues[_1] }
end
$queues = 4.times.map { Queue.new }
$threads = $queues.map.with_index do |queue, thread_id|
  "Thread #{thread_id}".instance_eval do
    Thread.new do
      queue.deq
      binding.irb
      $queues[thread_id] = nil
      $queues[next_thread_id].enq :pass if next_thread_id
    end
  end
end
$queues[0].enq :pass
$threads.map(&:join)
$ ruby file.rb
irb(Thread 0):001> pass 1
irb(Thread 1):001> pass 2
irb(Thread 2):001> pass 3
irb(Thread 3):001> pass 0
irb(Thread 0):002> exit # This will restore MAIN_CONTEXT to nil
irb(Thread 1):002> pass 3
irb(Thread 3):002> exit
irb(Thread 1):003> pass 2
irb(Thread 2):002> exit
irb(Thread 1):004> exit
# Order: start 0, start 1, start 2, start 3, exit 0, exit 3, exit 2, exit 1

@tompng
Copy link
Member Author

tompng commented Apr 27, 2024

Restoring to nil may crash IRB.
I'll create more simple pull request first, and re-think later for fundamental solution.

@tompng tompng closed this Apr 27, 2024
@tompng tompng deleted the restore_current_context branch April 27, 2024 12:11
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

IRB doesn't restore context after leaving the nested session
1 participant