[PATCH] ruby: make sure the database is closed

[Ali Polatel]

2012/4/24 Felipe Contreras <felipe.contreras at gmail.com>:
> On Mon, Apr 23, 2012 at 11:43 PM, Ali Polatel <alip at exherbo.org> wrote:
>> 2012/4/23 Felipe Contreras <felipe.contreras at gmail.com>:
>>> On Mon, Apr 23, 2012 at 5:04 PM, Ali Polatel <alip at exherbo.org> wrote:
>>>
>>>> I'd rather not do this.
>>>> Please read: http://comments.gmane.org/gmane.comp.lang.ruby.general/320324
>>>
>>> OK, I've read this.. So?
>>
>> You are one step close to what I thought you had thought.
>
> I don't parse that. I meant that _now_ I've read it.

Sorry, my subconscious likes to barf on emails every now and then.

>>> The order in which Ruby's garbage-collector frees the database and
>>> other objects is irrelevant, because with this patch we are not
>>> manually freeing other objects, only the database.
>>
>> What I wanted was to make all objects "depending" on the database to be unusable
>> once the database object is freed. Seeing that's not practically easy
>> I decided to leave
>> it to the user.
>
> Well, sure, but if the user has problems, the user can keep the
> database object around.
>
> Personally I don't see why an object, like say a query would remain
> working correctly after the database is gone, either by calling
> .close() directly, or just loosing the pointer to the original object.
> I don't think users would expect that, or, even if they somehow found
> it useful, that most likely would be very seldom, and hardly worth
> worrying about it.

Working correctly is not expected but wouldn't it be more appropriate
to throw an exception rather than dumping core or printing on standard error?

>>> Sure, it's _better_ if the user calls close(), even better if it's
>>> inside an 'ensure', and even better if blocks are used (which I am
>>> using in most cases), but that's not *required*.
>>
>> If you have such a use case, I'm fine with that patch.
>> I might push it in the next few days or get someone else to push it.
>
> I did at some point, not sure if I do now. But that's beside the
> point, the point is that it really does happen.
>
>>> The user might just do:
>>>
>>> def foo
>>>  db = Notmuch::Database.new($db_name, :mode => Notmuch::MODE_READ_WRITE)
>>> end
>>>
>>> That's perfectly fine in Ruby (although not ideal), since 'db' will
>>> get garbage-collected. But nobody will be able to use the database
>>> again until that process is killed.
>>>
>>> You think that's correct?
>>
>> Yes that is correct. I have not thought about this.
>> I'd say it's a partial misunderstanding on my part due to lack of
>> (and/or too much) vodka.
>
> Well, there's only two options.
>
> a) This works:
>
>  def foo
>    db = Notmuch::Database.new($db_name, :mode => Notmuch::MODE_READ_WRITE)
>  end
>
> (as in; the database gets closed eventually)
>
> b) This works:
>
>  def start_query(search)
>    db = Notmuch::Database.new($db_name)
>    return db.query(search)
>  end
>
> (as in; the query returned would keep working properly)
>
> I personally don't see why anybody would want b), and if they have
> problems with code like that, I think it's obvious to see why.
>
> I think we should go for a) and thus apply the patch.

I wonder whether we can make both work somehow.
Maybe by using talloc explicitly and keeping reference pointers?
I don't know whether it's worth bothering.

> Cheers.

Cheers

> --
> Felipe Contreras

        -alip