Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix a GC compaction issue with busy_handler #466

Merged
merged 1 commit into from
Jan 9, 2024
Merged

Fix a GC compaction issue with busy_handler #466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 22 additions & 14 deletions ext/sqlite3/database.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@

VALUE cSqlite3Database;

static void
database_mark(void *ctx)
{
sqlite3RubyPtr c = (sqlite3RubyPtr)ctx;
rb_gc_mark(c->busy_handler);
}

static void
deallocate(void *ctx)
{
Expand All @@ -31,15 +38,13 @@ database_memsize(const void *ctx)
}

static const rb_data_type_t database_type = {
"SQLite3::Backup",
{
NULL,
deallocate,
database_memsize,
.wrap_struct_name = "SQLite3::Backup",
Copy link
Contributor

@fractaledmind fractaledmind Jan 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A question from ignorance:

Peter's article describes the wrap_struct_name this way:

This is a C string name we give for our TypedData object. This name is used for debugging and statistics purposes. Although there are no requirements for this name (it just has to be a valid C string that’s null-terminated), it is a good idea to give it an informative and unique name to make it easy for yourself and other developers to identify the object.

For my own edification, why do we use the same string for 3 different TypedData objects:

https://github.com/search?q=repo%3Asparklemotion%2Fsqlite3-ruby%20rb_data_type_t&type=code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea. I personally tend to set it as the name of the class it backs, but as Peter says, it can be anything.

Ultimately this doesn't have a big impact, the only way to see it is via ObjectSpace.dump and ObjectSpace.dump_all so is only useful when debugging memory related things.

Would probably be worth reworking them all, but I don't own that gem, so not my call.

.function = {
.dmark = database_mark,
.dfree = deallocate,
.dsize = database_memsize,
},
0,
0,
RUBY_TYPED_WB_PROTECTED, // Not freed immediately because the dfree function do IOs.
.flags = RUBY_TYPED_WB_PROTECTED, // Not freed immediately because the dfree function do IOs.
};

static VALUE
Expand Down Expand Up @@ -202,10 +207,11 @@ trace(int argc, VALUE *argv, VALUE self)
}

static int
rb_sqlite3_busy_handler(void *ctx, int count)
rb_sqlite3_busy_handler(void *context, int count)
{
VALUE self = (VALUE)(ctx);
VALUE handle = rb_iv_get(self, "@busy_handler");
sqlite3RubyPtr ctx = (sqlite3RubyPtr)context;

VALUE handle = ctx->busy_handler;
VALUE result = rb_funcall(handle, rb_intern("call"), 1, INT2NUM(count));

if (Qfalse == result) { return 0; }
Expand Down Expand Up @@ -240,11 +246,13 @@ busy_handler(int argc, VALUE *argv, VALUE self)
rb_scan_args(argc, argv, "01", &block);

if (NIL_P(block) && rb_block_given_p()) { block = rb_block_proc(); }

rb_iv_set(self, "@busy_handler", block);
ctx->busy_handler = block;

status = sqlite3_busy_handler(
ctx->db, NIL_P(block) ? NULL : rb_sqlite3_busy_handler, (void *)self);
ctx->db,
NIL_P(block) ? NULL : rb_sqlite3_busy_handler,
(void *)ctx
);

CHECK(ctx->db, status);

Expand Down
1 change: 1 addition & 0 deletions ext/sqlite3/database.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

struct _sqlite3Ruby {
sqlite3 *db;
VALUE busy_handler;
};

typedef struct _sqlite3Ruby sqlite3Ruby;
Expand Down