Share support

[jscheid]

Hi, thanks for maintaining this package. I'd like to add support for the share interface - that is,

• a Share struct representing a share handle, with functions to configure it,
• the ability to add a Share instance to an Easy instance, or remove it,
• a mechanism to ensure share handles are kept around as long as they're referenced by any Easy handles.

Would you accept a PR for this, and are you interested in discussing API design and implementation details?

[sagebind]

The Share API is pretty unsafe, as you have to bring your own mutexes and manually keep track of references to avoid use-after-free. It might be doable, but would be tricky I think. It would have to be implemented in a way that:

• Does not require the user to know about or use the Share API
• Doesn't add unnecessary mutexes if the user isn't using the Share API
• Doesn't require any backwards-incompatible changes
• Is totally safe, of course

Given those constraints I don't see any reason why we wouldn't accept this to be added to the API, as it could be useful in some scenarios.

---

For design, I messed with this API several years ago, though I never finished it. Probably the most realistic approach would be to use an Arc internally in our Share wrapper. The API we present looks like you add Easy handles to it, but under the hood you're really adding a clone of the Share reference to the Easy handle. So each Easy2 struct would have an Option<Share> of some kind, to prevent the Share from being dropped until all Easy handles are.

Both CURLSHOPT_LOCKFUNC and CURLSHOPT_UNLOCKFUNC must be set by our Share wrapper unconditionally, since presumably users will want to continue to use their Easy handles as Send.

If users want a lock-free Share for single-threaded use, it may be better to create a totally separate LocalShare type that has different constraints. It would have to act more like Multi does today, taking ownership of the Easy handles and returning a reduced version that is !Send. I don't know if there's much of a use-case for this though.

The docs are not clear whether it is safe to set CURLSHOPT_SHARE after some Easy handles have already started using it. It may be better to require the aspects to share be set once in the constructor and not allow it to be changed again.

As a user, I might expect using the API to perhaps look something like this:

let share = Share::builder()
    .dns()
    .psl()
    .build();

let easy1 = Easy::new();
let easy2 = Easy::new();

easy1.set_share(&share);
easy2.set_share(&share);

Note that the docs do indicate that the following can not be used in a multi-threaded context, even if locking is configured:

• CURL_LOCK_DATA_COOKIE
• CURL_LOCK_DATA_SSL_SESSION
• CURL_LOCK_DATA_CONNECT
• CURL_LOCK_DATA_HSTS

I guess that does make me wonder if actually a LocalShare would be more interesting...

[jscheid]

Thanks for getting back to me so quickly, and for sharing your thoughts. The API design looks very similar to what I had in mind.

I guess that does make me wonder if actually a LocalShare would be more interesting...

Yes - after opening this issue I came across this message which actually makes it sounds like offering LocalShare is more useful (or rather, less misleading) than a thread-safe share.

Given this, let me first run some tests to see how much value this adds over the caching built into multi. Specifically I'm wondering if DNS and SSL contexts are cached by multi - the documentation isn't entirely clear on that if I remember correctly.

[sagebind]

Yes, certain elements are already shared between handles that are added to the same multi, which IMO also limits the usefulness of the Share API if Multi does some of the same things. The Everything Curl book says:

All easy handles added to the same multi handle automatically share connection cache and dns cache.

This does start to refresh my memory of the last time I interacted with this API, and decided it actually wasn't all that useful.