[1.9] String.split hungs JRuby on broken UTF-8 string.

In 1.6.2 and 1.6.3 versions bug is present too.

Sorry, I can't find how to edit first message, to add this info in header

A very good find! Investigating!

This appears to be spinning inside Joni, so the fix may have to come there:

"main" prio=5 tid=4303618048 nid=0x114604000 runnable [4636798976]
   java.lang.Thread.State: RUNNABLE
	at org.joni.SearchAlgorithm$7.search(SearchAlgorithm.java:479)
	at org.joni.Matcher.forwardSearchRange(Matcher.java:129)
	at org.joni.Matcher.search(Matcher.java:432)
	at org.jruby.RubyString.regexSplit19(RubyString.java:4484)
	at org.jruby.RubyString.splitCommon19(RubyString.java:4454)
	at org.jruby.RubyString.split19(RubyString.java:4404)
	at org.jruby.RubyString$i$split19.call(RubyString$i$split19.gen:65535)
	at org.jruby.runtime.invokedynamic.InvokeDynamicSupport.fail(InvokeDynamicSupport.java:1166)
	at java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:566)
	at org.jruby.runtime.invokedynamic.InvokeDynamicSupport.invocationFallback(InvokeDynamicSupport.java:469)
	at Users.headius.Downloads.test_string_split.block_4$RUBY$__file__(/Users/headius/Downloads/test_string_split.rb:14)

Bug still present both in JRuby 1.6.4 and JRuby 1.7.0 snapshot from 26.08.2011.

jruby 1.7.0.dev (ruby-1.9.2-p136) (2011-08-26 d7a6a2f) (Java HotSpot(TM) Client
VM 1.6.0_21) [Windows 7-x86-java]

Same here with JRuby 1.6.5:

$ export LC_ALL=en_US.utf8

$ jruby --1.9 --version
jruby 1.6.5 (ruby-1.9.2-p136) (2011-10-25 9dcd388) (Java HotSpot(TM) Client VM 1.6.0_29) [linux-i386-java]

$ jruby --1.9 test_string_split.rb 

Result: 100% CPU usage.

May not be totally optimal, but all Joni tests pass. Only tested on Windows.

Actually, this may not throw the ArgumentError as expected in the supplied unit test. I'll look into it a bit more.

Throwing an exception this time It will still need to be caught upstream in JRuby, but this prevents the infinite loop

Dave,

Looks like JRuby still needs to catch org.joni.exception.ValueException and raise ArgumentError. Do you want to take a crack at this part, too?

Yes, I just wanted someone to look at the Joni patch first before I continued. Thanks.

Patch attached. I catch the exception if it's a UTF error, otherwise I just re-throw it. Not sure if this is the best solution, but the provided unit test seems to work (on Windows).

Note: This patch will only work with the patched version of Joni.

The patch looks reasonable to me. Current specs pass. We'll need a spec for it, but that should be easy, based on the test case here.

Submitted a pull request to the RubySpec project in https://github.com/rubyspec/rubyspec/pull/104

Patch provided, Hiro thinks it looks good. I am not reviewing it right now, but marking as blocker for 1.6.6 so we get it in. Thank you!

Ok, I attempted to apply the patch to Joni, but it no longer matches. Did I miss something? I also talked to Marcin about it and he said that the MRI version of Oniguruma does not have the same check at that point.

In general we have tried to keep the codebases similar if not identical, so that changes over time can easily be integrated. So there's some reason why MRI doesn't hang on this case and we don't that we haven't found. The change you made is logical, and I made a similar modification that applied ok, but I share Marcin's concern about patching Oniguruma in ways that do not exist in MRI to match MRI behavior.

Hmm.

Charlie,

The patches are unfortunately not numbered. Apply the patch in the following order:

[system] joni [git:master] $ git am --signoff < 0001-Possible-fix-for-JRUBY-5808.patch
Applying: Possible fix for JRUBY-5808
[system] joni [git:master] $ git am --signoff < 0001-Throw-an-exception-when-an-invalid-UTF-8-char-detect.patch
Applying: Throw an exception when an invalid UTF-8 char detected

I verified that this works on the current master's HEAD.

So as fate would have it, it seems that other fixes have solved the original issue with ActiveSuport rdoc (or at least, triggered a different error that appears to be unrelated to encodings) and so Marcin was able to re-enable a coderange check that makes the provided test case pass too. I'm calling this resolved and will open a separate bug for the new rdoc issue.

CR coderange check on master@f91cb6b and jruby-1_6@a6a7fe6.

Ok, after fixing JRUBY-6359, I discovered this hang was still present. And I found the cause.

Numerous places in our code were calling the 1.8 versions of RubyRegexp methods rather than the 1.9 versions. This obviously caused problems, including this hang, because it did not use encoding-aware regexp logic.

I have committed a fix for that (and several similar cases I found while auditing) to master@03ba524 and jruby-1_6@a26b926.