AuthenticationMiddleware was called already panic

[dneuhaeuser-zalando]

We are observing that our actix-web application occasionally panics under a lot of contention with the message "AuthenticationMiddleware was called already". This seems to come from here https://github.com/actix/actix-web-httpauth/blob/v0.4.0/src/middleware.rs#L184

We have recently migrated to actix-web 2.0 and actix-web-httpauth 0.4. We haven't observed this before the migration.

As this doesn't happen for all requests, there seems to be some race condition either within actix-web or actix-web-httpauth that causes this.

We use bearer token authorization and within the validation function, to validate the token, we just perform an HTTP request with reqwest and depending on the result return Ok or Err.

[stiiifff]

@dneuhaeuser-zalando Do you use a blocking operation in one of your handlers ?
I had the same issue and it came from the fact I was using the web::block helper function from actix-web in an async handler ...

[dneuhaeuser-zalando]

Yes and no. We don't use web::block. However we are performing database queries with diesel and those are of course blocking. (Not ideal, I know. We should change it at some point.)

[robjtede]

@dneuhaeuser-zalando can you provide us with a minimal reproduction program so we can trace the cause of the panic.

To be clear, web::block is non-blocking; its purpose is to run blocking code asynchronously on a background thread pool.

[stiiifff]

@robjtede re. web::blocking being non-blocking, that was my understanding ... but its use somehow lead to panics described in this issue. It would happen systematically if just 2 requests would arrive in //.

I replaced the use of web::block by an async move { ... } block and no more panics.
For testing, I simply used apache bench (e.g. ab -c 10 -n 100 http://127.0.0.1:8080/).

[boydjohnson]

It looks to be inherent in the contract between poll_ready and call. The implementation of Service in actix-web-httpauth assumes that poll_ready and call won't be called at the same time:

actix-extras/actix-web-httpauth/src/middleware.rs

Lines 183 to 185 in 6b5215d

	.try_lock()
	.expect("AuthenticationMiddleware was called already")
	.poll_ready(ctx)

This doesn't seem to be true given the comments on Service:
https://github.com/actix/actix-net/blob/a5c185e80e56d44ea88b4485729abdefb08b1c2a/actix-service/src/lib.rs#L95

This repo has code that reproduces the panic: https://github.com/boydjohnson/actix-example.

I have a commit that uses the futures::lock::Mutex that wraps the service and first calls poll on the lock and then poll_ready on the service. I would make a PR if it is wanted. boydjohnson@43f7fcf
----- EDIT 05-10-2020 -----
I think the above mentioned commit is not right and the commit by @stiiiifff stiiifff@a8f142d that swaps out the Mutex for a RefCell is the right way to go.
----- EDIT later 05-10-2020 ----
The commit by @stiiiifff stiiifff@a8f142d on further testing gives an intermittent BorrowRefError.
I have a commit that tries a different futures aware lock: boydjohnson@5d2cdf1 that is promising.

[stiiifff]

@boydjohnson Thx for further looking into this, I still have some intermittent errors in my project due to this issue. I will try out your proposed fix.

[boydjohnson]

I have had good luck with the debug-2 branch on github.com/boydjohnson/actix-extras for about a month without a panic and without a deadlock. I am going to make a PR with this branch today. @stiiifff did you try the debug-2 branch at github.com/boydjohnson/actix-extras?

[boydjohnson]

Here is the backtrace of the panic with debug symbols on.
https://gist.github.com/boydjohnson/8ab7d1aa1aadc9562ec50e67a3107fa6.

-- Edited this comment so as not to have a long back trace in the thread.

[cetra3]

@dneuhaeuser-zalando There is a PR active by @boydjohnson that should address your issue.

Are you able to test that the PR solves your issue?

[stiiifff]

As mentioned in the PR, it looks good on my end as well.