Is it possible to drop `Context` without leaking memory?

[dpc]

I'm looking for a way to be able to drop Context before starting (but after initi) without leaking anything. Right now it seems Box<FnBox<...>> will leak if the Context is dropped.

[dpc]

BTW. Is https://github.com/zonyitoo/context-rs/blob/master/src/context.rs#L417 this Box<Box<FnBox()>> going to get dropped at the end of init_fn if we just jump out the Context? I wonder if the fact if Context::load() is diverging or not is making any difference.

[dpc]

After more investigation, I don't understand why is the split between "arg, init and start" even necessary. From my perspective all I really want is a Context that will start executing a given extern C function with one pointer/usize argument. After that the extern C function is free to transmute the argument into anything.

To test this I've wrote:

dpc@b4a8e9b

And then I've modified mioco to use it:

https://github.com/dpc/mioco/blob/context-unboxed/src/lib.rs#L708

It seems to work, and thanks to this, I can just pass a reference to something that is guaranteed to out-live this Context. But if I never start the Context nothing bad happens.

Also the calling convention is very simple, and does not have to be kept in sync with context-rs.

[zonyitoo]

Indeed, the Box<Box<FnBox()>> will leak in this circumstance.

The InitFn may be necessary, because when the context is initializing, you may have to set the InitFn as a call back function to become a bridge to call the actual Box<Box<FnBox()>>.

In your code, you just use the InitFn as the actual callback function... Hmm, that is one special case.

[dpc]

It seems to me that Box<Box<FnBox()>> will leak in every circumstances. Even in the normal case, the end of scope of init_fn will not be ever reached, so Box<...> won't drop.

How is that a special case? It seems to me like any case can be handled by extern C fn + arg : usize.

[zonyitoo]

You could drop it like this

let ret = {
    let func = Box::from_raw(...);
    func()
};

You create a fn instead of closure, so ... it won't be able to support closures.

[dpc]

Yes. That would work. OK.

But for my final requirement I don't want any resources that require drop inside init_fn. Could something like dpc@b4a8e9b get merged? Do you see anything that I've missed? It seems to me in such form, it is safe to drop Context before starting without anything leaking. And it saves one allocation.

[dpc]

It can support closures. In mioco I just pass arg as a reference to struct Closure containg given Context:

https://github.com/dpc/mioco/blob/context-unboxed/src/lib.rs#L708

The actual closure goes to:
https://github.com/dpc/mioco/blob/context-unboxed/src/lib.rs#L424

so the C function itself can:

https://github.com/dpc/mioco/blob/context-unboxed/src/lib.rs#L719

[dpc]

Additional plus is, that context-rs wouldn't have to use unstable FnBox, getting closer to work on stable. If anyone needs it, they can use FnBox scheme on their own.

[zonyitoo]

Seems reasonable ... Hmm

[dpc]

Hi, so what's the plan about it? RIght now I'm using a git path in mioco pointing to a fork with my patch. I'd like to be able to publish something on crates.io soon.

[zonyitoo]

Since the simd crate is still on Github, I can't publish this to crates.io.

I don't have time to think about the proper APIs of what you suggested. You may fire an PR and I will merge it to the master branch. Or I will do this when I find the time. :)

But there still are some thing has to be done before it gets "stable":

1. The assembly code in src/asm has to be tested on all the platforms, and it has to be rewritten with Intel style because VC on Windows cannot compile.
2. The simd crate is still unstable.

[zonyitoo]

Well, I have changed my mind, I should start it right now.

[dpc]

ATM The "stable" part is not immediate goal, I guess. But it's something to keep in mind. At some point it would be nice to have everything working on stable rust.

I have also figured out how to not depend on this feature in mioco.

However I still think that passing just one usize is a good change:

• the API will be the simplest I could imagine, thus it should never have to change again;
• Context will be dropable until started without leaking resources
• anyone can use any approach they want; just transmute the usize to a pointer to anything;
• eventually it could work on stable;

[dpc]

Just saw your update. 👍

[zonyitoo]

So, the usize parameter should be removed?

[zonyitoo]

You should check this out, is that what you want?

[dpc]

usize removed? What I wanted is that init_with takes only one C function, and one argument (usize or void*).

See:

dpc@b4a8e9b

[zonyitoo]

Done in the latest commit on branch feature-rawptr.

[zonyitoo]

But actually, you could just use init_with and provide ptr::null_mut() as the start parameter. It is all the same.

[zonyitoo]

I am going to revert it.

[dpc]

OK, the init_with with null start would work for me just fine.

[zonyitoo]

Merged.