Skip to content
This repository was archived by the owner on Dec 4, 2023. It is now read-only.

Commit d969b1c

Browse files
committed
add back the basics of memory testing.
This adds back the memory tests which ensure The Ruby Racer's stability. It also includes the changes needed to make these initial specs pass. At this point to disallow calling Dispose() directly on the underlying Isolate. If you do, then you could have all of the Ruby objects happily sitting in memory, but trying to use them would result in a segfault for trying to use a dead isolate. In order to avoid that, we'd have to put in safeguards everywhere to make sure for any operation on any context or object, that it's isolate was still alive. In order to reduce the complexity of memory management, the safest thing is to let the garbage collection hook dispose of the isolate itself. That way there is no doubt that the isolate is no longer in use.
1 parent 78ac679 commit d969b1c

4 files changed

Lines changed: 30 additions & 17 deletions

File tree

.travis.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ script:
2424
- bundle exec rake compile
2525
- bundle exec rspec spec/c
2626
- bundle exec rspec spec/v8/context_spec.rb
27+
- bundle exec rspec spec/mem
2728
sudo: false
2829
addons:
2930
apt:

ext/v8/isolate.cc

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ namespace rr {
1111
ClassBuilder("Isolate").
1212
defineSingletonMethod("New", &New).
1313

14-
defineMethod("Dispose", &Isolate::Dispose).
14+
defineMethod("IdleNotificationDeadline", &IdleNotificationDeadline).
1515

1616
store(&Class);
1717
}
@@ -26,17 +26,17 @@ namespace rr {
2626
create_params.array_buffer_allocator = &data->array_buffer_allocator;
2727
v8::Isolate* isolate = v8::Isolate::New(create_params);
2828

29-
3029
isolate->SetData(0, data);
3130
isolate->AddGCPrologueCallback(&clearReferences);
3231

3332
data->isolate = isolate;
3433
return Isolate(isolate);
3534
}
3635

37-
VALUE Isolate::Dispose(VALUE self) {
36+
37+
VALUE Isolate::IdleNotificationDeadline(VALUE self, VALUE deadline_in_seconds) {
3838
Isolate isolate(self);
39-
isolate->Dispose();
40-
return Qnil;
39+
Locker lock(isolate);
40+
return Bool(isolate->IdleNotificationDeadline(NUM2DBL(deadline_in_seconds)));
4141
}
4242
}

ext/v8/isolate.h

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ namespace rr {
5050
*/
5151
static void destroy(IsolateData* data) {
5252
Isolate isolate(data);
53+
isolate->Dispose();
5354
isolate.decrementTotalReferences();
5455
}
5556

@@ -159,7 +160,15 @@ namespace rr {
159160
while (data->rb_release_queue.try_dequeue(object)) {
160161
isolate.releaseObject(object);
161162
}
162-
rb_gc_mark(data->retained_objects);
163+
//TODO: This should not be necessary since sometimes the
164+
//instance of V8::RetainedObjects appears to magically be of
165+
//type T_NONE instead of T_OBJECT. Later, it will be T_OBJECT,
166+
//but if called while T_NONE, it will cause rb_gc_mark to dump
167+
//core.
168+
//See https://bugs.ruby-lang.org/issues/10803
169+
if (TYPE(data->retained_objects) != T_NONE) {
170+
rb_gc_mark(data->retained_objects);
171+
}
163172
}
164173

165174
/**
@@ -177,7 +186,7 @@ namespace rr {
177186
}
178187

179188

180-
static VALUE Dispose(VALUE self);
189+
static VALUE IdleNotificationDeadline(VALUE self, VALUE deadline_in_seconds);
181190

182191
/**
183192
* Recent versions of V8 will segfault unless you pass in an

spec/mem/blunt_spec.rb

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,36 +7,39 @@
77
end
88
#allocate a single context to make sure that v8 loads its snapshot and
99
#we pay the overhead.
10-
V8::Context.new
10+
V8::Isolate.new
1111
@start_memory = process_memory
1212
GC.stress = true
1313
end
1414

1515
after do
1616
GC.stress = false
1717
end
18+
19+
it "can create 3 isolates in a row" do
20+
3.times { V8::Isolate.new }
21+
end
22+
1823
it "won't increase process memory by more than 50% no matter how many contexts we create" do
19-
500.times do
20-
V8::Context.new
21-
run_v8_gc
24+
250.times do
25+
isolate = V8::Context.new.isolate.native
26+
isolate.IdleNotificationDeadline(0.1)
2227
end
23-
process_memory.should <= @start_memory * 1.5
28+
expect(process_memory).to be <= @start_memory * 1.5
2429
end
2530

2631
it "can eval simple value passing statements repeatedly without significantly increasing memory" do
27-
V8::C::Locker() do
28-
cxt = V8::Context.new
32+
V8::Context.new do |cxt|
2933
500.times do
3034
cxt.eval('7 * 6')
31-
run_v8_gc
35+
cxt.isolate.native.IdleNotificationDeadline(0.1)
3236
end
3337
end
34-
process_memory.should <= @start_memory * 1.1
38+
expect(process_memory).to be <= @start_memory * 1.1
3539
end
3640

3741
def process_memory
3842
/\w*[ ]*#{Process.pid}[ ]*([.,\d]*)[ ]*([.,\d]*)[ ]*([\d]*)[ ]*([\d]*)/.match(`ps aux`)[4].to_i
3943
end
4044

4145
end
42-

0 commit comments

Comments
 (0)