Hi,

At Sat, 26 May 2007 14:29:55 +0900,
Gonzalo Garramuno wrote in [ruby-core:11252]:
> Personally, I'd keep Init_stack() and ruby_init_stack(), but
> expose only ruby_init_stack() in ruby.h (as is now).
> I'd add a new int (boolean) parameter to ruby_init_stack()
> to allow avoiding the STACK_CHECK().  This would break
> backwards compatability of this function, but as it has only
> recently been introduced that should not be too terrible.
> I'd merge Init_stack() to call ruby_init_stack() to avoid
> code duplication.

It does solve nothing.

The point of this problem is that ruby interpreter is not
thread-safe as a library, so that you must not call it in
native-threads other than the native-thread on which ruby
itself is running.  Instead, you have to send any requests to
the native-thread and process them in it.

> Finally, I'd probably remove the 1Mb clamping of the stack
> space that is used for win32/linux (not sure what its
> purpose is, either -- seems like if you are reading
> getrlimit values you should use them as is).

To try to catch stack overflow.

> A better patch would involve merging Init_stack and
> ruby_init_stack by having one call the other.

Sounds reasonable.

> For ruby1.9:
> 
> I'm not too familiar with the insides of ruby1.9 yet, so my
> question first is how does each thread keep knowledge of its
> own stack?  
> Currently the gc.c in ruby1.9 seems almost identical to 1.8
> in this aspect, so I'm a tad puzzled how this is working at
> all now.  It seems like YARV as it is right now is not safe
> at all for embedding.

It does know about the native-threads which are created in
ruby.  Therefore, you can't store VALUEs on the stacks of other
threads.


Index: eval.c =================================================================== --- eval.c (revision 12390) +++ eval.c (working copy) @@ -1330,5 +1330,4 @@ char **rb_origenviron; void rb_call_inits _((void)); -void Init_stack _((VALUE*)); void Init_heap _((void)); void Init_ext _((void)); Index: gc.c =================================================================== --- gc.c (revision 12390) +++ gc.c (working copy) @@ -1451,7 +1451,19 @@ ruby_set_stack_size(size) void -Init_stack(addr) +ruby_init_stack(addr +#ifdef __ia64 + , bsp +#endif + ) VALUE *addr; +#ifdef __ia64 + void *bsp; +#endif { + size_t size = 0; +#if defined(_WIN32) || defined(__CYGWIN__) + MEMORY_BASIC_INFORMATION m; +#endif + #ifdef __ia64 if (rb_gc_register_stack_start == 0) { @@ -1468,14 +1480,10 @@ Init_stack(addr) # endif } - { - VALUE *bsp = (VALUE*)rb_ia64_bsp(); - if (rb_gc_register_stack_start == 0 || - bsp < rb_gc_register_stack_start) { - rb_gc_register_stack_start = bsp; - } + if (rb_gc_register_stack_start == 0 || + bsp < rb_gc_register_stack_start) { + rb_gc_register_stack_start = bsp; } #endif #if defined(_WIN32) || defined(__CYGWIN__) - MEMORY_BASIC_INFORMATION m; memset(&m, 0, sizeof(m)); VirtualQuery(&m, &m, sizeof(m)); @@ -1500,61 +1508,22 @@ Init_stack(addr) rb_gc_stack_start = addr; #endif -#ifdef HAVE_GETRLIMIT + + /* set STACK_LEVEL_MAX */ +#if defined(_WIN32) || defined(__CYGWIN__) + size = (char *)mi.BaseAddress - (char *)mi.AllocationBase; +#elif defined HAVE_GETRLIMIT { struct rlimit rlim; if (getrlimit(RLIMIT_STACK, &rlim) == 0) { - unsigned int space = rlim.rlim_cur/5; - - if (space > 1024*1024) space = 1024*1024; - STACK_LEVEL_MAX = (rlim.rlim_cur - space) / sizeof(VALUE); + size = rlim.rlim_cur; } } #endif -} - -void ruby_init_stack(VALUE *addr -#ifdef __ia64 - , void *bsp -#endif - ) -{ - if (!rb_gc_stack_start || - STACK_UPPER(&addr, - rb_gc_stack_start > addr, - rb_gc_stack_start < addr)) { - rb_gc_stack_start = addr; + if (size) { + size_t space = size / 5; + if (space > 1024*1024) space = 1024*1024; + STACK_LEVEL_MAX = (size - space) / sizeof(VALUE); } -#ifdef __ia64 - if (!rb_gc_register_stack_start || - (VALUE*)bsp < rb_gc_register_stack_start) { - rb_gc_register_stack_start = (VALUE*)bsp; - } -#endif -#ifdef HAVE_GETRLIMIT - { - struct rlimit rlim; - - if (getrlimit(RLIMIT_STACK, &rlim) == 0) { - unsigned int space = rlim.rlim_cur/5; - - if (space > 1024*1024) space = 1024*1024; - STACK_LEVEL_MAX = (rlim.rlim_cur - space) / sizeof(VALUE); - } - } -#elif defined _WIN32 - { - MEMORY_BASIC_INFORMATION mi; - DWORD size; - DWORD space; - - if (VirtualQuery(&mi, &mi, sizeof(mi))) { - size = (char *)mi.BaseAddress - (char *)mi.AllocationBase; - space = size / 5; - if (space > 1024*1024) space = 1024*1024; - STACK_LEVEL_MAX = (size - space) / sizeof(VALUE); - } - } -#endif } Index: ruby.h =================================================================== --- ruby.h (revision 12390) +++ ruby.h (working copy) @@ -581,13 +581,13 @@ VALUE rb_require _((const char*)); #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()); +#define Init_stack(addr) ruby_init_stack(addr, rb_ia64_bsp()) #else void ruby_init_stack(VALUE*); +#define Init_stack(addr) ruby_init_stack(addr) +#endif #define RUBY_INIT_STACK \ VALUE variable_in_this_stack_frame; \ - ruby_init_stack(&variable_in_this_stack_frame); -#endif + Init_stack(&variable_in_this_stack_frame); + void ruby_init _((void)); void ruby_options _((int, char**));
-- Nobu Nakada