Upgrading actix-web

[antiblue]

Hi all, I started working on upgrading the Actix-Web parts, but I stumble over an issue with Tokio.

Through OPCUASession.connect the function get_server_endpoints_from_url<T> is called and at the end an instance of Session is dropped.
This causes a panic, because Tokio wants to enter a blocking region:

thread 'actix-rt|system:0|arbiter:3' panicked at 'Cannot drop a runtime in a context where blocking is not allowed. This happens when a runtime is dropped from within an asynchronous context.', ...tokio-1.32.0\src\runtime\blocking\shutdown.rs:51:21

I have found running-actix-web-using-tokiomain in Actix-Web's documentation, stating that block_in_place will not work. Unfortunately the backtrace reveals exactly that:

  13: core::ptr::drop_in_place<opcua::client::session::session::Session>
             at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be\library\core\src\ptr\mod.rs:497
  14: opcua::client::client::Client::get_server_endpoints_from_url<ref$<str$> >
             at ...\opcua\lib\src\client\client.rs:493

Any suggestions on how to fix this?

(The code has been forked here)

[AiyionPrime]

@antiblue I'm struggling with the same problem. Was just drafting a different example structure in #355, when I hit this error as well. Did you make any progress with this?
I'd really like to see the example having the same actix version as the rest of the code.

[antiblue]

No, sorry.

[AiyionPrime]

After reading through the other PRs and recent commits, maybe @einarmo might be a good choice to ask for directions about this problem.

[einarmo]

You shouldn't be having a trouble with a runtime being dropped in the client now, since the client no longer contains a runtime. Is this hitting somewhere else?

[AiyionPrime]

I encountered the problem when I drafted an update of samples/web-client to the actix version in the lib crate. I'll provide a PR tomorrow; today's kind of full; sorry.

[vlnzrv]

Hi @AiyionPrime! Any updates about this PR with the actix version upgrade?

[AiyionPrime]

Hey @vlnzrv, sorry the topic has spread over various PRs and issues now.

I talked to the maintainer @locka99 a week ago, where he made clear that he was going to look into how to enable external support of this library.
But he also remarked his currently quite limited time, of which he can't spend as much in this project.

That and the amount of open PRs (about 18) have led me to stop opening PRs for the moment, until we find a way to reduce (and hopefully merge some) again.

My goal is to help this project, not drown it in work.
Nevertheless that's what I've already told @einarmo in a different context and was not aware others were awaiting this as well.

[vlnzrv]

Hey @AiyionPrime, thanks for the update and your contributions! Some comments why I was looking for a way to update it. In May rust nightly broke for old versions of time crate: rust-lang/rust#125319. And it's not going to be fixed according to that thread, so the only way around is to upgrade time crate. Opcua crate depends on time crate via actix-web, so it's a blocker for an update. In a couple of weeks rust stable will stop working for these old versions of time crate, so It'll be an annoying issue.

[AiyionPrime]

@vlnzrv Yeah I saw that, too.
It was not meant as "won't do" :)
I opened a draft PR #360 to discuss the shortcomings of my implementation.

One definitely is the non graceful shutdown during ctrl+c which results in the runtime getting terminated in async context. I think I have an idea for that though.

My code is not tested at all; I've got an interview to setup now, but will get back to this later in the evening.
Hope this helps anyway.

[AiyionPrime]

@vlnzrv 1.80.0 is happening next Thursday, isn't it?

[AiyionPrime]

Furthermore it's only the sample which is running the ancient version of time, if I'm not mistaken?
So temporarily dropping it would be a bad, but available fallback, I think.

[vlnzrv]

Yes, it's only a sample, because dependency in the lib itself was updated. In the worst case the sample could be just commented in Cargo.toml until it's fixed. It won't break unit / integration tests.

[vlnzrv]

.80.0 is happening next Thursday, isn't it?

Correct: https://releases.rs/docs/1.80.0/

[AiyionPrime]

Alright. The PR I drafted to address the beta issue is #361. And the draft to verify the CI addition works is #362.

@vlnzrv Furthermore for your downstream repo you should be fine performing the same addition to the Cargo.toml in case this does not get merged in time.