[BUG] Token refresh is not thread-safe

[fsy2001]

The authentication token for FDUHole needs to be refreshed periodically, but currently, there's no mechanisms to prevent multiple requests from trying to refresh the token at the same time. This may lead to occasional 401 errors from some APIs.

This problem can be fixed by adding a readers–writer lock, as described in #12.

[singularity-s0]

How about we just make everything serial for now? We can optimize the implementation later.

[fsy2001]

LGTM.

[singularity-s0]

Is there any structured way of making things serial?

[fsy2001]

I believe httpMaximumConnectionsPerHost of URLSessionConfiguration is suitable for this.

[singularity-s0]

I believe httpMaximumConnectionsPerHost of URLSessionConfiguration is suitable for this.

As I have said, being single-threaded doesn't mean there are no longer any concurrency problems. Consider the following scenario:

A and B wants to obtain tickets from UIS. A and B start requests with the same stored cookie. Both requests enter a queue. A's request goes first and finds that the cookie has expired. A starts another request to re-logins to UIS. However, B's request will start before A's re-login request because it's at the front of the queue. So B will send another useless request only to find out (again) its token has expired. And B also starts another request to re-login to UIS.

Although only one request is sent at a time, the total number of requests remain the same. The concurrency problem still remains.

[w568w]

By the way, what is wrong with a read-write lock?

I were thinking the mechanism I had proposed before was both correct and easy-to-implement...?

And read-write lock itself is also easy to implement with either a semaphore or a mutex.

FYI here is a piece of pseudo-code:

int requestWithToken() {
    lock(read);
    cur_token = get_token();
    err = request(cur_token);
    unlock(read);
    if (err == null) return OK;
    lock(write);
    if (get_token() != cur_token) unlock(write), return ALREADY_REFRESHED_BY_OTHERS;
    set_token(request_token());
    unlock(write);
    return REFRESHED;
}

[fsy2001]

I believe httpMaximumConnectionsPerHost of URLSessionConfiguration is suitable for this.

As I have said, being single-threaded doesn't mean there are no longer any concurrency problems. Consider the following scenario:

A and B wants to obtain tickets from UIS. A and B start requests with the same stored cookie. Both requests enter a queue. A's request goes first and finds that the cookie has expired. A starts another request to re-logins to UIS. However, B's request will start before A's re-login request because it's at the front of the queue. So B will send another useless request only to find out (again) its token has expired. And B also starts another request to re-login to UIS.

Although only one request is sent at a time, the total number of requests remain the same. The concurrency problem still remains.

I think you are off-topic. This issue has nothing to do with UIS service.

[singularity-s0]

I think you are off-topic. This issue has nothing to do with UIS service.

This is only an example. The same thing happens with JWT refresh.

[singularity-s0]

And read-write lock itself is also easy to implement with either a semaphore or a mutex.

The problem is that Swift has nearly none of the async concurrency primitives. We have to implement them ourselves.

[fsy2001]

By the way, what is wrong with a read-write lock?

We'll implement this eventually, but there might not be enough time before our first release. We are discussing a temporary workaround.

I am not available for development until next week.

[singularity-s0]

This article claims we could use Dispatch Barriers to deal with our readers-writers problem. Perhaps we should take a look.

[w568w]

Swift has nearly none of the async concurrency primitives

😿