Forum: Ruby-core [ruby-trunk - Bug #7046][Open] ERB#run and ERB#result are not safe for concurrent use

F1d37642fdaa1662ff46e4c65731e9ab?d=identicon&s=25 Charles Nutter (headius)
on 2012-09-22 01:00
(Received via mailing list)
Issue #7046 has been reported by headius (Charles Nutter).

----------------------------------------
Bug #7046: ERB#run and ERB#result are not safe for concurrent use
https://bugs.ruby-lang.org/issues/7046

Author: headius (Charles Nutter)
Status: Open
Priority: Normal
Assignee:
Category: lib
Target version:
ruby -v: 2.0.0.dev


ERB#run and ERB#result both accept an optional binding under which to
execute the template. However, if none is given, they both use
TOPLEVEL_BINDING by default. Given that by default, the _erbout variable
is used for the String into which ERB output gets appended, this causes
concurrent template execution on the same thread or separate threads to
modify the same buffer. On JRuby, this led to overflow errors when
in-progress writes saw their buffers suddenly altered.

This also causes any variables or values evaluated at TOPLEVEL to remain
referenced.

I have provided a patch (https://gist.github.com/3764377) that is still
very close to the toplevel binding, but instead uses the following logic
each call to get a new, isolated binding in which to run the template:

eval "proc{binding}.call", TOPLEVEL_BINDING

This provides visibility to all values at TOPLEVEL, isolates runs to
reduce concurrency issues, and guarantees any values stored in the
binding will be thrown away after execution.

This fix should be backported to 1.9.3 at minimum.
F1d37642fdaa1662ff46e4c65731e9ab?d=identicon&s=25 Charles Nutter (headius)
on 2012-09-22 01:01
(Received via mailing list)
Issue #7046 has been updated by headius (Charles Nutter).


Redmine did not parse the patch URL correctly. It is
https://gist.github.com/3764377
----------------------------------------
Bug #7046: ERB#run and ERB#result are not safe for concurrent use
https://bugs.ruby-lang.org/issues/7046#change-29652

Author: headius (Charles Nutter)
Status: Open
Priority: Normal
Assignee:
Category: lib
Target version:
ruby -v: 2.0.0.dev


ERB#run and ERB#result both accept an optional binding under which to
execute the template. However, if none is given, they both use
TOPLEVEL_BINDING by default. Given that by default, the _erbout variable
is used for the String into which ERB output gets appended, this causes
concurrent template execution on the same thread or separate threads to
modify the same buffer. On JRuby, this led to overflow errors when
in-progress writes saw their buffers suddenly altered.

This also causes any variables or values evaluated at TOPLEVEL to remain
referenced.

I have provided a patch (https://gist.github.com/3764377) that is still
very close to the toplevel binding, but instead uses the following logic
each call to get a new, isolated binding in which to run the template:

eval "proc{binding}.call", TOPLEVEL_BINDING

This provides visibility to all values at TOPLEVEL, isolates runs to
reduce concurrency issues, and guarantees any values stored in the
binding will be thrown away after execution.

This fix should be backported to 1.9.3 at minimum.
F1d37642fdaa1662ff46e4c65731e9ab?d=identicon&s=25 Charles Nutter (headius)
on 2012-09-22 18:57
(Received via mailing list)
Issue #7046 has been updated by headius (Charles Nutter).


Oh, I also want to make it clear that this is broken on MRI too, for
both the nested, same-thread case and the separate, concurrent thread
cases. It needs to be patched.
----------------------------------------
Bug #7046: ERB#run and ERB#result are not safe for concurrent use
https://bugs.ruby-lang.org/issues/7046#change-29676

Author: headius (Charles Nutter)
Status: Open
Priority: Normal
Assignee:
Category: lib
Target version:
ruby -v: 2.0.0.dev


ERB#run and ERB#result both accept an optional binding under which to
execute the template. However, if none is given, they both use
TOPLEVEL_BINDING by default. Given that by default, the _erbout variable
is used for the String into which ERB output gets appended, this causes
concurrent template execution on the same thread or separate threads to
modify the same buffer. On JRuby, this led to overflow errors when
in-progress writes saw their buffers suddenly altered.

This also causes any variables or values evaluated at TOPLEVEL to remain
referenced.

I have provided a patch (https://gist.github.com/3764377) that is still
very close to the toplevel binding, but instead uses the following logic
each call to get a new, isolated binding in which to run the template:

eval "proc{binding}.call", TOPLEVEL_BINDING

This provides visibility to all values at TOPLEVEL, isolates runs to
reduce concurrency issues, and guarantees any values stored in the
binding will be thrown away after execution.

This fix should be backported to 1.9.3 at minimum.
F1d6cc2b735bfd82c8773172da2aeab9?d=identicon&s=25 Nobuyoshi Nakada (nobu)
on 2012-09-24 08:46
(Received via mailing list)
Hi,

At Sat, 22 Sep 2012 07:59:51 +0900,
headius (Charles Nutter) wrote in [ruby-core:47638]:
> I have provided a patch (https://gist.github.com/3764377) that is still very
close to the toplevel binding, but instead uses the following logic each call to
get a new, isolated binding in which to run the template:
>
> eval "proc{binding}.call", TOPLEVEL_BINDING

`TOPLEVEL_BINDING.dup' would work too.
F1d37642fdaa1662ff46e4c65731e9ab?d=identicon&s=25 Charles Nutter (headius)
on 2012-09-24 19:59
(Received via mailing list)
Issue #7046 has been updated by headius (Charles Nutter).


In JRuby it does not appear that dup'ing a binding copies all structures
over, so we'd need to fix that as well to use TOPLEVEL_BINDING.dup.

It appears we match 1.8.7 behavior still, for Binding#dup:


system ~/projects/jruby $ ruby-1.8.7-p358 -e "eval 'a = 1',
TOPLEVEL_BINDING.dup; eval 'puts a', TOPLEVEL_BINDING.dup"
1

system ~/projects/jruby $ jruby -e "eval 'a = 1', TOPLEVEL_BINDING.dup;
eval 'puts a', TOPLEVEL_BINDING.dup"
1

system ~/projects/jruby $ ruby-1.9.3 -e "eval 'a = 1',
TOPLEVEL_BINDING.dup; eval 'puts a', TOPLEVEL_BINDING.dup"
<main>:in `<main>': undefined local variable or method `a' for
main:Object (NameError)
  from -e:1:in `eval'
  from -e:1:in `<main>'

Given that we would not be releasing patched ERB in any release other
than one with this fixed, I think TOPLEVEL_BINDING.dup is probably the
simplest way.
----------------------------------------
Bug #7046: ERB#run and ERB#result are not safe for concurrent use
https://bugs.ruby-lang.org/issues/7046#change-29716

Author: headius (Charles Nutter)
Status: Open
Priority: Normal
Assignee:
Category: lib
Target version:
ruby -v: 2.0.0.dev


ERB#run and ERB#result both accept an optional binding under which to
execute the template. However, if none is given, they both use
TOPLEVEL_BINDING by default. Given that by default, the _erbout variable
is used for the String into which ERB output gets appended, this causes
concurrent template execution on the same thread or separate threads to
modify the same buffer. On JRuby, this led to overflow errors when
in-progress writes saw their buffers suddenly altered.

This also causes any variables or values evaluated at TOPLEVEL to remain
referenced.

I have provided a patch (https://gist.github.com/3764377) that is still
very close to the toplevel binding, but instead uses the following logic
each call to get a new, isolated binding in which to run the template:

eval "proc{binding}.call", TOPLEVEL_BINDING

This provides visibility to all values at TOPLEVEL, isolates runs to
reduce concurrency issues, and guarantees any values stored in the
binding will be thrown away after execution.

This fix should be backported to 1.9.3 at minimum.
F1d6cc2b735bfd82c8773172da2aeab9?d=identicon&s=25 Nobuyoshi Nakada (nobu)
on 2012-09-25 03:25
(Received via mailing list)
Hi,

At Tue, 25 Sep 2012 02:58:32 +0900,
headius (Charles Nutter) wrote in [ruby-core:47676]:
> It appears we match 1.8.7 behavior still, for Binding#dup:

1.8.7 is dying.
F1d37642fdaa1662ff46e4c65731e9ab?d=identicon&s=25 Charles Nutter (headius)
on 2012-09-25 17:27
(Received via mailing list)
Issue #7046 has been updated by headius (Charles Nutter).


Indeed, and we will fix our 1.9 mode to work like 1.9.3.
----------------------------------------
Bug #7046: ERB#run and ERB#result are not safe for concurrent use
https://bugs.ruby-lang.org/issues/7046#change-29735

Author: headius (Charles Nutter)
Status: Open
Priority: Normal
Assignee:
Category: lib
Target version:
ruby -v: 2.0.0.dev


ERB#run and ERB#result both accept an optional binding under which to
execute the template. However, if none is given, they both use
TOPLEVEL_BINDING by default. Given that by default, the _erbout variable
is used for the String into which ERB output gets appended, this causes
concurrent template execution on the same thread or separate threads to
modify the same buffer. On JRuby, this led to overflow errors when
in-progress writes saw their buffers suddenly altered.

This also causes any variables or values evaluated at TOPLEVEL to remain
referenced.

I have provided a patch (https://gist.github.com/3764377) that is still
very close to the toplevel binding, but instead uses the following logic
each call to get a new, isolated binding in which to run the template:

eval "proc{binding}.call", TOPLEVEL_BINDING

This provides visibility to all values at TOPLEVEL, isolates runs to
reduce concurrency issues, and guarantees any values stored in the
binding will be thrown away after execution.

This fix should be backported to 1.9.3 at minimum.
F1d37642fdaa1662ff46e4c65731e9ab?d=identicon&s=25 Charles Nutter (headius)
on 2012-09-25 18:29
(Received via mailing list)
Issue #7046 has been updated by headius (Charles Nutter).


We are shipping the proc-based fix in JRuby 1.7.0.RC1 today. There's a
good chance we'll fix the Binding#dup behavior for 1.7.0, in which case
we'd use the TOPLEVEL_BINDING.dup fix.

Functionally, I think they should both appear to be the same to any
user.
----------------------------------------
Bug #7046: ERB#run and ERB#result are not safe for concurrent use
https://bugs.ruby-lang.org/issues/7046#change-29736

Author: headius (Charles Nutter)
Status: Open
Priority: Normal
Assignee:
Category: lib
Target version:
ruby -v: 2.0.0.dev


ERB#run and ERB#result both accept an optional binding under which to
execute the template. However, if none is given, they both use
TOPLEVEL_BINDING by default. Given that by default, the _erbout variable
is used for the String into which ERB output gets appended, this causes
concurrent template execution on the same thread or separate threads to
modify the same buffer. On JRuby, this led to overflow errors when
in-progress writes saw their buffers suddenly altered.

This also causes any variables or values evaluated at TOPLEVEL to remain
referenced.

I have provided a patch (https://gist.github.com/3764377) that is still
very close to the toplevel binding, but instead uses the following logic
each call to get a new, isolated binding in which to run the template:

eval "proc{binding}.call", TOPLEVEL_BINDING

This provides visibility to all values at TOPLEVEL, isolates runs to
reduce concurrency issues, and guarantees any values stored in the
binding will be thrown away after execution.

This fix should be backported to 1.9.3 at minimum.
F1d37642fdaa1662ff46e4c65731e9ab?d=identicon&s=25 Charles Nutter (headius)
on 2012-10-14 21:48
(Received via mailing list)
Issue #7046 has been updated by headius (Charles Nutter).


Ping! I am wondering if a decision has been made about how to fix this.
As mentioned, JRuby is shipping the proc-based fix.
----------------------------------------
Bug #7046: ERB#run and ERB#result are not safe for concurrent use
https://bugs.ruby-lang.org/issues/7046#change-30663

Author: headius (Charles Nutter)
Status: Open
Priority: Normal
Assignee:
Category: lib
Target version:
ruby -v: 2.0.0.dev


ERB#run and ERB#result both accept an optional binding under which to
execute the template. However, if none is given, they both use
TOPLEVEL_BINDING by default. Given that by default, the _erbout variable
is used for the String into which ERB output gets appended, this causes
concurrent template execution on the same thread or separate threads to
modify the same buffer. On JRuby, this led to overflow errors when
in-progress writes saw their buffers suddenly altered.

This also causes any variables or values evaluated at TOPLEVEL to remain
referenced.

I have provided a patch (https://gist.github.com/3764377) that is still
very close to the toplevel binding, but instead uses the following logic
each call to get a new, isolated binding in which to run the template:

eval "proc{binding}.call", TOPLEVEL_BINDING

This provides visibility to all values at TOPLEVEL, isolates runs to
reduce concurrency issues, and guarantees any values stored in the
binding will be thrown away after execution.

This fix should be backported to 1.9.3 at minimum.
F24ff61beb80aa5f13371aa22a35619c?d=identicon&s=25 mame (Yusuke Endoh) (Guest)
on 2012-11-05 13:50
(Received via mailing list)
Issue #7046 has been updated by mame (Yusuke Endoh).

Status changed from Open to Assigned
Assignee set to nobu (Nobuyoshi Nakada)
Target version set to 2.0.0

Looks good to me.  Nobu, do you have any concern?

--
Yusuke Endoh <mame@tsg.ne.jp>
----------------------------------------
Bug #7046: ERB#run and ERB#result are not safe for concurrent use
https://bugs.ruby-lang.org/issues/7046#change-32406

Author: headius (Charles Nutter)
Status: Assigned
Priority: Normal
Assignee: nobu (Nobuyoshi Nakada)
Category: lib
Target version: 2.0.0
ruby -v: 2.0.0.dev


ERB#run and ERB#result both accept an optional binding under which to
execute the template. However, if none is given, they both use
TOPLEVEL_BINDING by default. Given that by default, the _erbout variable
is used for the String into which ERB output gets appended, this causes
concurrent template execution on the same thread or separate threads to
modify the same buffer. On JRuby, this led to overflow errors when
in-progress writes saw their buffers suddenly altered.

This also causes any variables or values evaluated at TOPLEVEL to remain
referenced.

I have provided a patch (https://gist.github.com/3764377) that is still
very close to the toplevel binding, but instead uses the following logic
each call to get a new, isolated binding in which to run the template:

eval "proc{binding}.call", TOPLEVEL_BINDING

This provides visibility to all values at TOPLEVEL, isolates runs to
reduce concurrency issues, and guarantees any values stored in the
binding will be thrown away after execution.

This fix should be backported to 1.9.3 at minimum.
8cbb39dadafaf2287a83a13ee4981ec9?d=identicon&s=25 usa (Usaku NAKAMURA) (Guest)
on 2012-12-19 12:51
(Received via mailing list)
Issue #7046 has been updated by usa (Usaku NAKAMURA).

Status changed from Closed to Rejected

I once backported r37594 at r38318, but it broke rubyspec.
----------------------------------------
Backport #7046: ERB#run and ERB#result are not safe for concurrent use
https://bugs.ruby-lang.org/issues/7046#change-34850

Author: headius (Charles Nutter)
Status: Rejected
Priority: Normal
Assignee: usa (Usaku NAKAMURA)
Category:
Target version:


ERB#run and ERB#result both accept an optional binding under which to
execute the template. However, if none is given, they both use
TOPLEVEL_BINDING by default. Given that by default, the _erbout variable
is used for the String into which ERB output gets appended, this causes
concurrent template execution on the same thread or separate threads to
modify the same buffer. On JRuby, this led to overflow errors when
in-progress writes saw their buffers suddenly altered.

This also causes any variables or values evaluated at TOPLEVEL to remain
referenced.

I have provided a patch (https://gist.github.com/3764377) that is still
very close to the toplevel binding, but instead uses the following logic
each call to get a new, isolated binding in which to run the template:

eval "proc{binding}.call", TOPLEVEL_BINDING

This provides visibility to all values at TOPLEVEL, isolates runs to
reduce concurrency issues, and guarantees any values stored in the
binding will be thrown away after execution.

This fix should be backported to 1.9.3 at minimum.
This topic is locked and can not be replied to.