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

oj: pthread_mutex_init is unsupported! #1566

Closed
deepj opened this issue Feb 4, 2019 · 31 comments
Closed

oj: pthread_mutex_init is unsupported! #1566

deepj opened this issue Feb 4, 2019 · 31 comments
Assignees
Milestone

Comments

@deepj
Copy link

deepj commented Feb 4, 2019

After solving several issues in #1437 there is a new one

This is reproducible on TruffleRuby 1.0.0 RC12

To reproduce:

require 'oj'

Error:

ERROR: pthread_mutex_init is unsupported!
truffleruby: org.graalvm.polyglot.PolyglotException: com.oracle.truffle.llvm.runtime.LLVMExitException
Original Internal Error:
com.oracle.truffle.llvm.runtime.LLVMExitException
@eregon
Copy link
Member

eregon commented Feb 4, 2019

@deepj Thank you for the report, I filed GR-13707 internally for the Sulong team to resolve this.

@deepj
Copy link
Author

deepj commented Mar 6, 2019

@eregon Any news with this? Since the report is not public so I'm asking.

@chrisseaton
Copy link
Collaborator

I think Sulong aren't planning pthread support soon. Perhaps we should provide some of our own shims for things like mutexes and condition variables (why doesn't oj use the Ruby ones?!)

@deepj
Copy link
Author

deepj commented Mar 10, 2019

@ohler55 ^^ maybe interesting for you

@ohler55
Copy link

ohler55 commented Mar 10, 2019

For historical reasons or just because I'm familiar with pthread. At once point pthreads had less overhead but that was many years ago.

@enebo
Copy link
Contributor

enebo commented Aug 9, 2019

I was excited to see how well you ran oj because you site says 3.7 is running 98.90% tests pass of the oj test suite: https://www.graalvm.org/docs/reference-manual/compatibility/#oj

It immediately hit this issue locally for me. Is there some way you work around this to test oj? Seems like your results would be 0% otherwise.

@chrisseaton
Copy link
Collaborator

I think the immediate answer is that HAVE_LIBPTHREAD is undefined. That's what the log shows me from this test run. I'm not sure who undefined it of it it's undefined by default. Maybe a config run fails because pthread_mutex_init doesn't work and so undefines HAVE_LIBPTHREAD? But then I'm not sure why you wouldn't see that locally.

@enebo
Copy link
Contributor

enebo commented Aug 9, 2019

@chrisseaton Just to let you know more details I am on fedora core 29 and I just did a gem install oj --version 3.7.12 (3.8 is out but I hit this and realized you advertised running on 3.7). If you test system does not have libpthread then it sets HAVE_LIBPTHREAD false/undefined and compiles with out it? That is why it works there but fails on my system. I did I understand that correctly?

@chrisseaton
Copy link
Collaborator

It looks more like the configuration test fails and so doesn't set the feature flag.

mkdir -p tmp/x86_64-linux/oj/2.6.2
cd tmp/x86_64-linux/oj/2.6.2
/opt/graalvm/jre/languages/ruby/bin/truffleruby -I. ../../../../ext/oj/extconf.rb
>>>>> Creating Makefile for truffleruby version 2.6.2 on x86_64-linux <<<<<
checking for rb_time_timespec()... yes
checking for rb_ivar_count()... yes
checking for rb_ivar_foreach()... yes
checking for stpcpy()... yes
checking for rb_data_object_wrap()... yes
creating Makefile
cd -
cd tmp/x86_64-linux/oj/2.6.2
/usr/bin/gmake
compiling ../../../../ext/oj/cache8.c
compiling ../../../../ext/oj/circarray.c
compiling ../../../../ext/oj/code.c
compiling ../../../../ext/oj/compat.c
compiling ../../../../ext/oj/custom.c
compiling ../../../../ext/oj/dump.c
compiling ../../../../ext/oj/dump_compat.c
compiling ../../../../ext/oj/dump_leaf.c
compiling ../../../../ext/oj/dump_object.c
compiling ../../../../ext/oj/dump_strict.c
compiling ../../../../ext/oj/err.c
compiling ../../../../ext/oj/fast.c
compiling ../../../../ext/oj/hash.c
compiling ../../../../ext/oj/hash_test.c
compiling ../../../../ext/oj/mimic_json.c
compiling ../../../../ext/oj/object.c
compiling ../../../../ext/oj/odd.c
compiling ../../../../ext/oj/oj.c
compiling ../../../../ext/oj/parse.c
compiling ../../../../ext/oj/rails.c
compiling ../../../../ext/oj/reader.c
../../../../ext/oj/reader.c:11:5: warning: 'NEEDS_UIO' is not defined, evaluates to 0 [-Wundef]
#if NEEDS_UIO
    ^
1 warning generated.
compiling ../../../../ext/oj/resolve.c
../../../../ext/oj/resolve.c:9:5: warning: 'HAVE_LIBPTHREAD' is not defined, evaluates to 0 [-Wundef]
#if HAVE_LIBPTHREAD
    ^
1 warning generated.
compiling ../../../../ext/oj/rxclass.c
compiling ../../../../ext/oj/saj.c
compiling ../../../../ext/oj/scp.c
compiling ../../../../ext/oj/sparse.c
compiling ../../../../ext/oj/stream_writer.c
compiling ../../../../ext/oj/strict.c
compiling ../../../../ext/oj/string_writer.c
compiling ../../../../ext/oj/trace.c
compiling ../../../../ext/oj/util.c
compiling ../../../../ext/oj/val_stack.c
compiling ../../../../ext/oj/wab.c
linking shared-object oj/oj.su
cd -
mkdir -p tmp/x86_64-linux/stage/lib/oj
install -c tmp/x86_64-linux/oj/2.6.2/oj.su lib/oj/oj.su
cp tmp/x86_64-linux/oj/2.6.2/oj.su tmp/x86_64-linux/stage/lib/oj/oj.su
ERROR: pthread_mutex_init is unsupported!
Run options: --seed 6114

# Running:



Finished in 0.008473s, 0.0000 runs/s, 0.0000 assertions/s.

0 runs, 0 assertions, 0 failures, 0 errors, 0 skips
org.graalvm.polyglot.PolyglotException
stty: standard input: Inappropriate ioctl for device
Loaded suite test/json_gem/json_addition_test
Started
...........
Finished in 1.552 seconds.
-------------------------------------------------------------------------------
11 tests, 42 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
-------------------------------------------------------------------------------
7.09 tests/s, 27.06 assertions/s
stty: standard input: Inappropriate ioctl for device
Loaded suite test/json_gem/json_common_interface_test
Started
.................
Finished in 0.252 seconds.
-------------------------------------------------------------------------------
17 tests, 38 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
-------------------------------------------------------------------------------
67.46 tests/s, 150.79 assertions/s
stty: standard input: Inappropriate ioctl for device
Loaded suite test/json_gem/json_encoding_test
Started
....
Finished in 0.24700000000000003 seconds.
-------------------------------------------------------------------------------
4 tests, 195 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
-------------------------------------------------------------------------------
16.19 tests/s, 789.47 assertions/s
stty: standard input: Inappropriate ioctl for device
Loaded suite test/json_gem/json_fixtures_test
Started
..
Finished in 0.394 seconds.
-------------------------------------------------------------------------------
2 tests, 0 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
-------------------------------------------------------------------------------
5.08 tests/s, 0.00 assertions/s
stty: standard input: Inappropriate ioctl for device
Loaded suite test/json_gem/json_generator_test
Started
.....................
Finished in 0.20400000000000001 seconds.
-------------------------------------------------------------------------------
21 tests, 101 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
-------------------------------------------------------------------------------
102.94 tests/s, 495.10 assertions/s
stty: standard input: Inappropriate ioctl for device
Loaded suite test/json_gem/json_generic_object_test
Started
.....
Finished in 1.345 seconds.
-------------------------------------------------------------------------------
5 tests, 26 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
-------------------------------------------------------------------------------
3.72 tests/s, 19.33 assertions/s
stty: standard input: Inappropriate ioctl for device
Loaded suite test/json_gem/json_parser_test
Started
...F
===============================================================================
Failure: test_error_message_encoding(JSONParserTest)
test/json_gem/json_parser_test.rb:29:in `test_error_message_encoding'
     26:   def test_error_message_encoding
     27:     bug10705 = '[ruby-core:67386] [Bug #10705]'
     28:     json = ".\"\xE2\x88\x9A\"".force_encoding(Encoding::UTF_8)
  => 29:     e = assert_raise(JSON::ParserError) {
     30:       JSON::Ext::Parser.new(json).parse
     31:     }
     32:     assert_equal(Encoding::UTF_8, e.message.encoding, bug10705)

<JSON::ParserError> expected but was
<NameError(<uninitialized constant JSON::Ext>)>

diff:
?                                   JSON::ParserError
? NameError(<uninitialized constant              xt>)
===============================================================================
..........................
Finished in 3.152 seconds.
-------------------------------------------------------------------------------
30 tests, 1601 assertions, 1 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
96.6667% passed
-------------------------------------------------------------------------------
9.52 tests/s, 507.93 assertions/s
stty: standard input: Inappropriate ioctl for device
Loaded suite test/json_gem/json_string_matching_test
Started
.
Finished in 0.034 seconds.
-------------------------------------------------------------------------------
1 tests, 2 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
-------------------------------------------------------------------------------
29.41 tests/s, 58.82 assertions/s

##########################################################################################
ruby test/tests.rb && ruby test/tests_mimic.rb && ruby test/tests_mimic_addition.rb

##########################################################################################
REAL_JSON_GEM=1 bundle exec ruby -Itest test/json_gem/json_addition_test.rb

##########################################################################################
REAL_JSON_GEM=1 bundle exec ruby -Itest test/json_gem/json_common_interface_test.rb

##########################################################################################
REAL_JSON_GEM=1 bundle exec ruby -Itest test/json_gem/json_encoding_test.rb

##########################################################################################
REAL_JSON_GEM=1 bundle exec ruby -Itest test/json_gem/json_ext_parser_test.rb

##########################################################################################
REAL_JSON_GEM=1 bundle exec ruby -Itest test/json_gem/json_fixtures_test.rb

##########################################################################################
REAL_JSON_GEM=1 bundle exec ruby -Itest test/json_gem/json_generator_test.rb

##########################################################################################
REAL_JSON_GEM=1 bundle exec ruby -Itest test/json_gem/json_generic_object_test.rb

##########################################################################################
REAL_JSON_GEM=1 bundle exec ruby -Itest test/json_gem/json_parser_test.rb

##########################################################################################
REAL_JSON_GEM=1 bundle exec ruby -Itest test/json_gem/json_string_matching_test.rb

@ohler55
Copy link

ohler55 commented Aug 9, 2019

Does truffle have a pthread library or an alternative that supports a mutex of some type? I'd be glad to make changes to improve support.

@chrisseaton
Copy link
Collaborator

This is a bit of a silly issue really - we could easily implement our own pthread_mutex_init - it's not complicated. We have been debating where it should be implemented and really we should just get on and do it in TruffleRuby for now. If you are keen to do it I can show you how.

@ohler55
Copy link

ohler55 commented Aug 9, 2019

Sure. Also need to know how to detect truffle is in use or at least some macro.

@chrisseaton
Copy link
Collaborator

I'll put together some notes and get back to you. You'd need to sign the OCA https://www.oracle.com/technetwork/community/oca-486395.html.

@ohler55
Copy link

ohler55 commented Aug 9, 2019

Signed and returned.

@eregon
Copy link
Member

eregon commented Aug 10, 2019

I just remembered, there is a C API for this: rb_nativethread_lock_* functions in ruby/thread_native.h. Advantages of those:

  • They are portable
  • They are already used by the OpenSSL C extension
  • TruffleRuby already implements them (using a Ruby Mutex currently)

This might be the easiest way.

@ohler55
Copy link

ohler55 commented Aug 10, 2019

I think you are right, that might be the easiest way to move forward.

@ohler55
Copy link

ohler55 commented Aug 10, 2019

On further digging it looks like the issue may be the macro Oj uses to detect the existence of pthread. I changed the extconf.rb file and the #if HAVE_LIBPTHREAD to do an explicit check for the pthread_mutex_init function. If not found Oj should revert to the Ruby Mutex. Please take a look at the 3.9alpha branch and see if that works for you.

@eregon eregon self-assigned this Aug 23, 2019
@eregon
Copy link
Member

eregon commented Aug 23, 2019

@ohler55 have_func('pthread_mutex_init') will still return true unfortunately, because Sulong defines it and even if it didn't the system libpthread does. We have to fix this on our side and remove the Sulong shim.

@ohler55
Copy link

ohler55 commented Aug 23, 2019

Is there any way of determining truffle is in use? I can special case truffle but I need to know how to detect it.

@eregon
Copy link
Member

eregon commented Aug 27, 2019

@ohler55 The TRUFFLERUBY macro is defined in C and the TruffleRuby constant is defined in Ruby. That's documented in https://github.com/oracle/truffleruby/blob/master/doc/user/truffleruby-additions.md#detecting-if-you-are-running-on-truffleruby

But we'd rather avoid TruffleRuby-specific code in gems if possible.

@ohler55
Copy link

ohler55 commented Aug 27, 2019

I prefer not having Truffle specific code as well but since the normal way of detecting function existence returns the wrong answer I don't see a way around it at this point other than always using the ruby mutex. I'll run some benchmarks to see what impact that has on performance.

@eregon
Copy link
Member

eregon commented Aug 27, 2019

@ohler55 I'm working on a fix to let pthread_mutex* just work. So that issue should be solved for the next release.

@ohler55
Copy link

ohler55 commented Aug 27, 2019

Great. I'll sit tight then.

@eregon
Copy link
Member

eregon commented Aug 27, 2019

The next error once pthread_mutex* works is:

$ bundle exec ruby -Ilib -roj -e 'p Oj.dump({"a":2})'
lib/truffle/truffle/cext.rb:195:in `__address__': RARRAY_PTRs cannot be converted to native pointers yet (RuntimeError)
	from /home/eregon/code/oj/ext/oj/dump.c:601:in `oj_dump_obj_to_json_using_params'
	from /home/eregon/code/oj/ext/oj/oj.c:1060:in `dump'
	from lib/truffle/truffle/cext_ruby.rb:37:in `dump'
	from -e:1:in `<main>'

Which is something we need to fix by either allowing RARRAY_PTRs to go native or finding another way to pass arguments to C extension functions taking (int argc, VALUE *argv, VALUE self).

@eregon
Copy link
Member

eregon commented Aug 27, 2019

On the upside, if I do allow RARRAY_PTRs to go native in a quick-and-dirty way just for this case, it does work:

$ truffleruby -roj -e 'puts Oj.dump({a: 2})'
{":a":2}

@enebo
Copy link
Contributor

enebo commented Aug 27, 2019

@eregon Any idea why you would see that error when you pass 98.8% of the specs in the gem tester? It looks like @chrisseaton was able to run all the tests above (maybe something changed recently in oj source?).

@eregon
Copy link
Member

eregon commented Aug 27, 2019

@enebo I think the output above is actually from the run of the module tester.
It looks like that run didn't actually run any Oj-specific tests due to pthread_mutex_init is unsupported when requiring oj, but still ran the test/json_gem tests and those probably ended up testing against the stdlib json gem.

ERROR: pthread_mutex_init is unsupported!
Run options: --seed 6114

# Running:
Finished in 0.008473s, 0.0000 runs/s, 0.0000 assertions/s.
0 runs, 0 assertions, 0 failures, 0 errors, 0 skips

org.graalvm.polyglot.PolyglotException
stty: standard input: Inappropriate ioctl for device
Loaded suite test/json_gem/json_addition_test

So I'd guess it's miscounted in this case, and somehow the tests were skipped with a failed require 'oj'. Unfortunate, but probably not so easy to detect from the output in a gem-agnostic manner.

@enebo
Copy link
Contributor

enebo commented Aug 27, 2019

@eregon ah ok yeah it does run MRI json tests. Mystery solved!

@enebo
Copy link
Contributor

enebo commented Aug 27, 2019

Err I should say it runs its tests on builtin json I think to make sure both oj and MRI passes the same set of stuff for json compatibility.

@aardvark179
Copy link
Contributor

I think we can fix things to allow RARRAY_PTR to be converted to a native pointer without too much trouble. We can achieve this by adding a native array storage strategy which would

  1. Allocate a block of memory large enough to hold all the array contents as VALUEs.
  2. Convert and copy the current array contents into this buffer.
  3. Send all ruby access to the array via conversion to and from the corresponding native VALUEs.
  4. Effectively GC mark all those values to prevent the objects from being collected.

There's a few other minor details that need to be handled (such as finding adjacent objects for ObjectSpace methods), but I have most of the implementation done. I'll add some specs to test RARRAY_PTR behaviour as well.

@eregon eregon added this to the 20.0.0 milestone Nov 22, 2019
@eregon
Copy link
Member

eregon commented Nov 22, 2019

Support for native RARRAY_PTR got merged in be190ac by @aardvark179.
I can confirm truffleruby -roj -e 'puts Oj.dump({a: 2})' works on master, so I'll close this and it will be in the next release.

We'll try to get nightly builds (#1483), so we can test TruffleRuby in oj's CI before the 20.0 release.

@eregon eregon closed this as completed Nov 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants