Project

General

Profile

Actions

Bug #19016

closed

syntax_suggest is not working with Ruby 3.2.0-preview2

Added by hsbt (Hiroshi SHIBATA) about 2 years ago. Updated about 2 years ago.

Status:
Closed
Target version:
[ruby-core:109995]

Description

syntax_suggest is merged as default gems in Ruby 3.2.0-preview2. But it's not working yet.

$ cat bar.rb
def foo
  def bar
end

$ ruby -v bar.rb
ruby 3.2.0dev (2022-09-22T05:37:56Z master f07e651a90) +YJIT [arm64-darwin22]
bar.rb:3: warning: mismatched indentations at 'end' with 'def' at 2
bar.rb:3: syntax error, unexpected end-of-input, expecting `end'

and gem version is also not working now.

$ cat foo.rb
require "bundler/inline"

gemfile do
  source "https://rubygems.org"
  gem "syntax_suggest"
end

require_relative "bar"
$ ruby -v foo.rb
ruby 3.2.0dev (2022-09-22T05:37:56Z master f07e651a90) +YJIT [arm64-darwin22]
/path/to/bar.rb:3: warning: mismatched indentations at 'end' with 'def' at 2
foo.rb:8:in `require_relative': /path/to/bar.rb:3: syntax error, unexpected end-of-input, expecting `end' (SyntaxError)
        from foo.rb:8:in `<main>'

But Ruby 3.1 is works.

$ ruby -v foo.rb
ruby 3.1.3p51 (2022-09-10 revision 9581248c4a) [arm64-darwin22]
/path/to/bar.rb:3: warning: mismatched indentations at 'end' with 'def' at 2
--> /path/to/bar.rb

Unmatched keyword, missing `end' ?

  1  def foo
❯ 2    def bar
  3  end

/Users/hsbt/.local/share/gem/gems/syntax_suggest-0.0.1/lib/syntax_suggest/core_ext.rb:93:in `require': /path/to/bar.rb:3: syntax error, unexpected end-of-input, expecting `end' (SyntaxError)
        from /Users/hsbt/.local/share/gem/gems/syntax_suggest-0.0.1/lib/syntax_suggest/core_ext.rb:93:in `require_relative'
        from foo.rb:8:in `<main>'
Actions #1

Updated by hsbt (Hiroshi SHIBATA) about 2 years ago

  • Description updated (diff)

Updated by hsbt (Hiroshi SHIBATA) about 2 years ago

I investigated this.

https://github.com/ruby/syntax_suggest/blob/main/lib/syntax_suggest/api.rb#L67

    Timeout.timeout(timeout) do
      record_dir ||= ENV["DEBUG"] ? "tmp" : nil
      search = CodeSearch.new(source, record_dir: record_dir).call
    end

This block raises #<ThreadError: can't alloc thread> with Ruby 3.2.0-preview2 and master branch. When I commented-out this Timeout.timeout, syntax_suggest is working fine.

I bisected timeout.rb. After https://github.com/ruby/ruby/commit/89fbec224d8e1fa35e82bf2712c5a5fd3dc06b83 happens this ThreadError with syntax_suggest. I'm not sure why timeout.rb raises ThreadError when using syntax_suggest.

Updated by mame (Yusuke Endoh) about 2 years ago

There are two different sub-issues in this issue.

1. syntax_suggest does not work for ruby foo.rb.

This is because a SyntaxError exception is not thrown even if there is a syntax error in foo.rb of ruby foo.rb. Instead, the interpreter prints the syntax error directly. So Exception#detailed_message is not called, which means syntax_suggest cannot intercept the exception handling.

This could be fixed by throwing a SyntaxError exception even if there is a parse error in foo.rb, but I don't know how hard this fix will be implemented.

2. syntax_suggest does not work for ruby -e 'load "foo.rb"'.

In this case, a SyntaxError exception is thrown, so Exception#detailed_message of syntax_suggest is actually called.
However, Exception#detailed_message is called after the interpreter termination process has started.
It is prohibited to create a new thread in this state, but syntax_suggest uses Timeout.timeout, which tries to create a new thread.
This causes the failure #<ThreadError: can't alloc thread> which @hsbt (Hiroshi SHIBATA) mentioned.

My PR changes the order so that Exception#detailed_message is called before the interpreter termination process.
However, I am a little afraid that this change has unforeseen side effects because it will print an error message when other threads are still running.

@ko1 (Koichi Sasada) pointed that the following script prints sub thread (RuntimeError) and then main thread (RuntimeError) in Ruby 3.1, and that main thread (RuntimeError) and then sub thread (RuntimeError) with my PR applied.

Thread.new { begin sleep; ensure; raise "sub thread"; end }
raise "main thread"
$ ruby -v tt.rb
ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [x86_64-linux]
#<Thread:0x00007ffff3f0b2e8 tt.rb:1 aborting> terminated with exception (report_on_exception is true):
tt.rb:1:in `ensure in block in <main>': sub thread (RuntimeError)
        from tt.rb:1:in `block in <main>'
tt.rb:2:in `<main>': main thread (RuntimeError)
$ ./miniruby -v tt.rb
ruby 3.2.0dev (2022-09-27T06:08:48Z error_print-before.. 4e8618306b) [x86_64-linux]
tt.rb:2:in `<main>': main thread (RuntimeError)
#<Thread:0x00007ffff7408fb8 tt.rb:1 aborting> terminated with exception (report_on_exception is true):
tt.rb:1:in `ensure in block in <main>': sub thread (RuntimeError)
        from tt.rb:1:in `block in <main>'

Updated by Eregon (Benoit Daloze) about 2 years ago

The current shutdown order in TruffleRuby is:

  1. execute main script (and save exception if any)
  2. run at_exit hooks
  3. print main script's exception with .full_message
  4. kill and wait all threads (which still runs some Ruby code in ensure)
  5. internal cleanups which do not need to run Ruby code

Which seems already the order with your change.
I think we shouldn't run any Ruby code after "kill and wait all threads" (because the kill might have caused inconsistent state and triggered in the middle of an ensure, etc), so I think this change makes a lot of sense.

Updated by mame (Yusuke Endoh) about 2 years ago

At the dev meeting, @nobu (Nobuyoshi Nakada) proposed a slightly different order of termination process: (1) calls #detailed_message and keep the result string, (2) terminates all threads, and (3) prints the kept message.

@nobu (Nobuyoshi Nakada) Do you create a patch?

Updated by Eregon (Benoit Daloze) about 2 years ago

Sounds OK-ish except I think full_message should be called instead of detailed_message.
Also if terminating all threads takes a while it delays the printing significantly, that's not so nice.
The order I posted above seems much cleaner.

Updated by schneems (Richard Schneeman) about 2 years ago

Thank you all for looking at this issue. I have some more cycles now. I took a look at the explanation and the PR. I don't feel equipped to weigh in on the implementation. I understand the problem description and proposed solution(s).

Updated by hsbt (Hiroshi SHIBATA) about 2 years ago

  • Assignee changed from hsbt (Hiroshi SHIBATA) to nobu (Nobuyoshi Nakada)

syntax_suggest is not working 3.2.0-preview3 too. @nobu (Nobuyoshi Nakada) will resolve this until preview4.

Actions #10

Updated by hsbt (Hiroshi SHIBATA) about 2 years ago

  • Target version set to 3.2
Actions #11

Updated by nobu (Nobuyoshi Nakada) about 2 years ago

  • Status changed from Assigned to Closed

Applied in changeset git|701dfe4eb741628213e4d701c13ad6d76904ac4f.


[Bug #19016] Handle syntax error in main script like other errors

So that SyntaxError#detailed_message will be used also in the case
exiting by such syntax error.

Actions

Also available in: Atom PDF

Like1
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0