mperham/memcache-client

Make continuum_native.rb conditional.

kstephens opened this issue · 5 comments

Issues:

  • We explicitly do not install compilers in our production web environments.
  • RubyInline has a race condition if $INLINEDIR is not unique for all child processes.
  • Creating unique $INLINEDIRs for each process is annoying.
  • But we do use RubyInline for some systems.

So....

I suggest that continuum_native.rb be enabled (or disabled) if some global variable or ENV var is set.
I can supply a patch if this is a reasonable idea.

Thanks,
KAS

Wouldn't it be better to fix the bug in RubyInline then?

What is the end result currently? Does memcache-client fail to start with some error?

There is a race-condition on the *.so that RubyInline generates. We've had to disable this kind of RubyInline usage before. So yea, RubyInline could be fixed, but thats far more difficult. What does memcache-client do if RubyInline is not installed? There doesn't appear to be a gem dependency on it.

Specifically the race-condition is that the same *.so file can be generated by RubyInline at the same time by more than one processes which might have different effective user ids.

I still don't understand what happens that is bad at runtime. Is it crashing? Not working?

RubyInline is completely optional - note the begin/rescue - as it only works on MRI. memcache-client is designed to work on MRI, Jruby, and any other Ruby VM. If the RubyInline optimized code in continuum_native.rb doesn't load/compile correctly, memcache-client will just use the pure Ruby method defined at the bottom of memcache.rb. If you are using multiple memcached servers, you'll see performance decline by about 10-15%. That's all that should happen.

I understand the performance implication. We do not run multiple memcacheds (even though we probably should). We've had segfaults due to RubyInline trashing its own *.so files, or creating them with incorrect or insecure permissions, and we don't install compilers in production for security reasons. I'll just disable it completely in our copy. Not a big deal. Thanks.