Re: if_ruby feature sometimes crash

[Bram Moolenaar]

Hiroshi Shirosaki wrote:

> This is originally reported here.
> https://bugs.ruby-lang.org/issues/8364
>
> Ruby crashes due to GC failure on mac osx. It seems ruby
> initialization in if_ruby.c is not a portable way.
> Stack start address should be prior to ruby stack region of every ruby
> eval for GC working correctly. It would be better to get the stack
> start address at startup timing of vim.
> Ruby 1.8.7 on Linux also has this issue.
>
> I've created a patch to fix this. Tested with ruby 1.8.7, 1.9.3 and
> trunk on osx.
> ruby_init_stack() also exists in ruby 1.8.7.
>
> https://gist.github.com/shirosaki/5547805

Thanks. Ruby does appear to have a special kind of GC.

--
[clop clop]
GUARD #1: Halt! Who goes there?
ARTHUR: It is I, Arthur, son of Uther Pendragon, from the castle of
Camelot. King of the Britons, defeator of the Saxons, sovereign of
all England!
GUARD #1: Pull the other one!
The Quest for the Holy Grail (Monty Python)

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[Yukihiro Nakadaira]

On Sat, May 11, 2013 at 8:47 PM, Bram Moolenaar <Br...@moolenaar.net> wrote:

Hiroshi Shirosaki wrote:

> This is originally reported here.
> https://bugs.ruby-lang.org/issues/8364
>
> Ruby crashes due to GC failure on mac osx. It seems ruby
> initialization in if_ruby.c is not a portable way.
> Stack start address should be prior to ruby stack region of every ruby
> eval for GC working correctly.  It would be better to get the stack
> start address at startup timing of vim.
> Ruby 1.8.7 on Linux also has this issue.
>
> I've created a patch to fix this. Tested with ruby 1.8.7, 1.9.3 and
> trunk on osx.
> ruby_init_stack() also exists in ruby 1.8.7.
>
> https://gist.github.com/shirosaki/5547805

Thanks.  Ruby does appear to have a special kind of GC.

ruby_init_stack() is also required for static linked version.

diff -r d73a4a163202 src/if_ruby.c
--- a/src/if_ruby.c    Sat May 11 13:56:18 2013 +0200
+++ b/src/if_ruby.c    Sat May 11 21:33:28 2013 +0900
@@ -716,12 +716,8 @@
         char *argv[] = {"gvim.exe"};
         NtInitialize(&argc, &argv);
 #endif
-        {
-#if defined(DYNAMIC_RUBY_VER) && DYNAMIC_RUBY_VER >= 18
-        ruby_init_stack(ruby_stack_start);
-#endif
-        ruby_init();
-        }
+        ruby_init_stack(ruby_stack_start);
+        ruby_init();
 #ifdef RUBY19_OR_LATER
         {
         int dummy_argc = 2;

--
Yukihiro Nakadaira - yukihiro....@gmail.com

[Yukihiro Nakadaira]

On Sat, May 11, 2013 at 9:49 PM, Yukihiro Nakadaira <yukihiro....@gmail.com> wrote:

On Sat, May 11, 2013 at 8:47 PM, Bram Moolenaar <Br...@moolenaar.net> wrote:

Hiroshi Shirosaki wrote:

> This is originally reported here.
> https://bugs.ruby-lang.org/issues/8364
>
> Ruby crashes due to GC failure on mac osx. It seems ruby
> initialization in if_ruby.c is not a portable way.
> Stack start address should be prior to ruby stack region of every ruby
> eval for GC working correctly.  It would be better to get the stack
> start address at startup timing of vim.
> Ruby 1.8.7 on Linux also has this issue.
>
> I've created a patch to fix this. Tested with ruby 1.8.7, 1.9.3 and
> trunk on osx.
> ruby_init_stack() also exists in ruby 1.8.7.
>
> https://gist.github.com/shirosaki/5547805

Thanks.  Ruby does appear to have a special kind of GC.

ruby_init_stack() is also required for static linked version.

And ruby_init_stack() is defined as

ruby-2.0-0-p0:
#ifdef __ia64
void ruby_init_stack(volatile VALUE*, void*);
#define ruby_init_stack(addr) ruby_init_stack((addr), rb_ia64_bsp())
#else
void ruby_init_stack(volatile VALUE*);
#endif

ruby-1.8.7-p371:
#ifdef __ia64
void ruby_init_stack(VALUE*, void*);
#define RUBY_INIT_STACK \
    VALUE variable_in_this_stack_frame; \
    ruby_init_stack(&variable_in_this_stack_frame, rb_ia64_bsp());
#else
void ruby_init_stack(VALUE*);
#define RUBY_INIT_STACK \
    VALUE variable_in_this_stack_frame; \
    ruby_init_stack(&variable_in_this_stack_frame);
#endif

So, We should handle __ia64.

[Hiroshi Shirosaki]

On Saturday, May 11, 2013 10:11:27 PM UTC+9, Yukihiro Nakadaira wrote:
> ruby_init_stack() is also required for static linked version.
>
> And ruby_init_stack() is defined as
>
> ruby-2.0-0-p0:
> #ifdef __ia64
> void ruby_init_stack(volatile VALUE*, void*);
> #define ruby_init_stack(addr) ruby_init_stack((addr), rb_ia64_bsp())
> #else
> void ruby_init_stack(volatile VALUE*);
>
> #endif
>
>
>
> ruby-1.8.7-p371:
> #ifdef __ia64
> void ruby_init_stack(VALUE*, void*);
> #define RUBY_INIT_STACK \
>     VALUE variable_in_this_stack_frame; \
>     ruby_init_stack(&variable_in_this_stack_frame, rb_ia64_bsp());
>
> #else
> void ruby_init_stack(VALUE*);
> #define RUBY_INIT_STACK \
>     VALUE variable_in_this_stack_frame; \
>     ruby_init_stack(&variable_in_this_stack_frame);
> #endif
>
>
>
> So, We should handle __ia64.
>

Thank you. I don't have __ia64 environment, but I think this would work.
Could you review the following?


diff --git a/src/if_ruby.c b/src/if_ruby.c
--- a/src/if_ruby.c
+++ b/src/if_ruby.c
@@ -153,16 +153,22 @@ static VALUE cVimWindow;
static VALUE eDeletedBufferError;
static VALUE eDeletedWindowError;

static int ensure_ruby_initialized(void);
static void error_print(int);
static void ruby_io_init(void);
static void ruby_vim_init(void);

+#ifdef __ia64
+#ifndef ruby_init_stack
+# define ruby_init_stack(addr) ruby_init_stack((addr), rb_ia64_bsp())
+#endif
+#endif
+
#if defined(DYNAMIC_RUBY) || defined(PROTO)
#ifdef PROTO
# define HINSTANCE int /* for generating prototypes */
#endif

/*
* Wrapper defines
*/
@@ -712,17 +718,17 @@ static int ensure_ruby_initialized(void)
#endif
#ifdef _WIN32
/* suggested by Ariya Mizutani */
int argc = 1;

char *argv[] = {"gvim.exe"};
NtInitialize(&argc, &argv);
#endif

{

-#if defined(DYNAMIC_RUBY_VER) && DYNAMIC_RUBY_VER >= 18

+#if defined(RUBY_VERSION) && RUBY_VERSION >= 18
ruby_init_stack(ruby_stack_start);
#endif

ruby_init();
}
#ifdef RUBY19_OR_LATER
{
int dummy_argc = 2;

char *dummy_argv[] = {"vim-ruby", "-e0"};

[Bram Moolenaar]

Yukihiro Nakadaira wrote:

> On Sat, May 11, 2013 at 8:47 PM, Bram Moolenaar <Br...@moolenaar.net> wrote:
>
> >
> > Hiroshi Shirosaki wrote:
> >
> > > This is originally reported here.
> > > https://bugs.ruby-lang.org/issues/8364
> > >
> > > Ruby crashes due to GC failure on mac osx. It seems ruby
> > > initialization in if_ruby.c is not a portable way.
> > > Stack start address should be prior to ruby stack region of every ruby
> > > eval for GC working correctly. It would be better to get the stack
> > > start address at startup timing of vim.
> > > Ruby 1.8.7 on Linux also has this issue.
> > >
> > > I've created a patch to fix this. Tested with ruby 1.8.7, 1.9.3 and
> > > trunk on osx.
> > > ruby_init_stack() also exists in ruby 1.8.7.
> > >
> > > https://gist.github.com/shirosaki/5547805
> >
> > Thanks. Ruby does appear to have a special kind of GC.
> >
>
> ruby_init_stack() is also required for static linked version.

But do we still need the check for DYNAMIC_RUBY_VER or another version
check?

Your other message states that it's different for 64 bits, thus I'm not
including this patch yet.


--
Nothing is impossible for the man who doesn't have to do it.

[Yukihiro Nakadaira]

On Sat, May 11, 2013 at 10:58 PM, Hiroshi Shirosaki <h.shi...@gmail.com> wrote:

Thank you. I don't have __ia64 environment, but I think this would work.
Could you review the following?

Thank you.

Here is small fix for dynamic load.

I don't have ia64 machine, too.

diff -r d73a4a163202 src/if_ruby.c
--- a/src/if_ruby.c    Sat May 11 13:56:18 2013 +0200

+++ b/src/if_ruby.c    Sat May 11 23:55:28 2013 +0900
@@ -227,7 +227,13 @@
 # define rb_float_new            dll_rb_float_new
 # define rb_ary_new            dll_rb_ary_new
 # define rb_ary_push            dll_rb_ary_push
-# define ruby_init_stack        dll_ruby_init_stack
+# ifdef __ia64
+#  define rb_ia64_bsp            dll_rb_ia64_bsp
+#  undef ruby_init_stack
+#  define ruby_init_stack(addr)        dll_ruby_init_stack((addr), rb_ia64_bsp())
+# else
+#  define ruby_init_stack        dll_ruby_init_stack
+# endif
 #else
 # define rb_str2cstr            dll_rb_str2cstr
 #endif
@@ -336,7 +342,12 @@
 static VALUE (*dll_rb_float_new) (double);
 static VALUE (*dll_rb_ary_new) (void);
 static VALUE (*dll_rb_ary_push) (VALUE, VALUE);
-static void (*ruby_init_stack)(VALUE*);
+# ifdef __ia64
+static void * (*dll_rb_ia64_bsp) (void);
+static void (*dll_ruby_init_stack)(VALUE*, void*);
+# else
+static void (*dll_ruby_init_stack)(VALUE*);
+# endif
 #endif
 #ifdef RUBY19_OR_LATER
 static VALUE (*dll_rb_int2big)(SIGNED_VALUE);
@@ -476,6 +487,9 @@
 #endif

 #if defined(DYNAMIC_RUBY_VER) && DYNAMIC_RUBY_VER >= 18

     {"rb_string_value_ptr", (RUBY_PROC*)&dll_rb_string_value_ptr},
+# ifdef __ia64
+    {"rb_ia64_bsp", (RUBY_PROC*)&dll_rb_ia64_bsp},
+# endif
     {"ruby_init_stack", (RUBY_PROC*)&dll_ruby_init_stack},
 # if DYNAMIC_RUBY_VER <= 19
     {"rb_float_new", (RUBY_PROC*)&dll_rb_float_new},
@@ -717,7 +731,7 @@

         NtInitialize(&argc, &argv);
 #endif
         {
-#if defined(DYNAMIC_RUBY_VER) && DYNAMIC_RUBY_VER >= 18
+#if defined(RUBY_VERSION) && RUBY_VERSION >= 18
         ruby_init_stack(ruby_stack_start);
 #endif
         ruby_init();

[Bram Moolenaar]

Yukihiro Nakadaira wrote:

> On Sat, May 11, 2013 at 10:58 PM, Hiroshi Shirosaki
> <h.shi...@gmail.com>wrote:
>
> > Thank you. I don't have __ia64 environment, but I think this would work.
> > Could you review the following?
>
> Thank you.
> Here is small fix for dynamic load.
> I don't have ia64 machine, too.

It looks OK to me, although I can't verify other combination of options.
Thanks!


--
Futility Factor: No experiment is ever a complete failure - it can always
serve as a negative example.