Services Kit: No locking around access to HTTP context information such as authentication

[stippi]

As time permits, I am trying to give the HTTP portion of the Services Kit a code review. Since I am not sure whether I find the time to investigate the problems I find more closely, or even fix them, I want to document my findings.

I think I have found one serious problem so far: There seems to be no locking around accessing the authentication per URL in the global HTTP context. This means there are several race conditions that can lead to memory corruption. There needs to be locking for accessing the hashmap of authentications per URL. And the authentication needs to be accessed via reference counting, so that the authentication object obtained in a HTTP thread can be used without the chance of it going away. This can happen when another HTTP thread replaces the authentication for a given URL. The old one needs to stay valid until all references are released.

[pulkomandy]

At least one more issue: the BUrlContext itself may be deleted while UrlRequests are still referencing it (unless the default context is used, this one is global static). Making it reference counted, and having the requests acquire a reference, would help with that.

Is it ok to use BReferenceable in public APIs? This sounds like a good way to handle BUrlContext and BHttpAuthentication lifetime.

[waddlesplash]

Can this be fixed in time for R1/alpha5?

[stippi]

For me personally, the crashes in the HTTP kit would be reason enough not to release. This issue cannot be the only one causing memory corruption, since HaikuDepot crashes since it uses the Kit, but it does not spawn concurrent HTTP requests. PulkoMandy seems to be working on it as we speak. As for myself - I have a wedding comming up and can't promise to find some time in the next weeks.

[stippi]

Replying to pulkomandy:

Is it ok to use BReferenceable in public APIs? This sounds like a good way to handle BUrlContext and BHttpAuthentication lifetime.

My personal opinion is that using it is long overdue. I've used it extensively in the new text stuff I designed for HaikuDepot and plan on proposing to be public API eventually.

[axeld]

The whole API is experimental, anyway, so just make it public (if not already), and use it.

[pulkomandy]

I believe nielx's rework of the kit has fixed this?

[nielx]

Locking is indeed front and center of the new kit design. I am going to close this ticket as no change required as it is unlikely there will be any more work done on the original kit.