Skip to content

Retry invalidated cache property get after cache is invalidated#1807

Draft
sander-machado wants to merge 3 commits into
z-galaxy:mainfrom
sander-machado:bugfix/retry-invalidated-cache
Draft

Retry invalidated cache property get after cache is invalidated#1807
sander-machado wants to merge 3 commits into
z-galaxy:mainfrom
sander-machado:bugfix/retry-invalidated-cache

Conversation

@sander-machado

Copy link
Copy Markdown

When a proxy would be created before the interface that is was targeting would be created, while caching is enabled, the cache would be permanently poisoned. You could solve it by recreating the client but I felt like it could be easier. The main issue is that once cache.ready is invalid it will never recover. An idea I had was that it would fall through to doing a real call even if the cache is invalid and if it succeeds would "refresh" the cache.

Property streams will still not work but at least property gets will, we encountered this while developing and it seemed like small enough change that might help some people in the future.

Disclaimer this PR was made with assistance of claude.

@sander-machado sander-machado changed the title Bugfix/retry invalidated cache Retry invalidated cache property get after cache is invalidated May 28, 2026
@sander-machado sander-machado force-pushed the bugfix/retry-invalidated-cache branch from 1ae37b3 to 3bd8b25 Compare May 28, 2026 15:37
@zeenix

zeenix commented May 29, 2026

Copy link
Copy Markdown
Contributor

@sander-machado thanks. Two questions upfront first:

@sander-machado

Copy link
Copy Markdown
Author

@sander-machado thanks. Two questions upfront first:

* Isn't this targetting [Update properties cache & notify property streams on service going up #1172](https://github.com/z-galaxy/zbus/issues/1172)? If so, please reference it in the commit message.

* Could you kindly take your existing PR ([🎨 zb: Remove unsafe type casting code #1439](https://github.com/z-galaxy/zbus/pull/1439) ) to completion first? 🙏
  • Didn't know there was a ticket for it, yeah it's the exact same. But reading the ticket made me think I should probably not wait for the user to try and do a property get but instead start a background task that subscribes to a known name event of the target interface + path. That should fix my issues + fix the stream issue what do you think
  • I'll have a look at it now

@sander-machado

Copy link
Copy Markdown
Author

@sander-machado thanks. Two questions upfront first:

* Isn't this targetting [Update properties cache & notify property streams on service going up #1172](https://github.com/z-galaxy/zbus/issues/1172)? If so, please reference it in the commit message.

* Could you kindly take your existing PR ([🎨 zb: Remove unsafe type casting code #1439](https://github.com/z-galaxy/zbus/pull/1439) ) to completion first? 🙏
* Didn't know there was a ticket for it, yeah it's the exact same. But reading the ticket made me think I should probably not wait for the user to try and do a property get but instead start a background task that subscribes to a known name event of the target interface + path. That should fix my issues + fix the stream issue what do you think

* I'll have a look at it now

But that approach probably only works if ObjectServer is used...

@sander-machado sander-machado marked this pull request as draft May 29, 2026 16:22
@zeenix

zeenix commented May 30, 2026

Copy link
Copy Markdown
Contributor

But that approach probably only works if ObjectServer is used...

I believe you meant ObjectManager. We can't assume that to exist on the service. You don't actually need to wait/look for the interface+object but rather only the service (i-e the name on the bus). If the interface/object are dynamic, the user needs to handle that themselves using the client API we provide for it.

@sander-machado

Copy link
Copy Markdown
Author

But that approach probably only works if ObjectServer is used...

I believe you meant ObjectManager. We can't assume that to exist on the service. You don't actually need to wait/look for the interface+object but rather only the service (i-e the name on the bus). If the interface/object are dynamic, the user needs to handle that themselves using the client API we provide for it.

Yeah good point, I'm thinking about a system with two fallbacks. If ObjectManager exists try to use that, otherwise if the service doesn't exist wait for it to appear and then finally just manually check on property get requests. Does that sound reasonable?

@zeenix

zeenix commented May 30, 2026

Copy link
Copy Markdown
Contributor

If ObjectManager exists try to use that

That would solve the problem at hand (properties cache) but user will still have a proxy to a non-existant interface+object and any method calls will fail and they've also no way to know the state of the properties cache.

Keeping that in mind, I'm starting to think that perhaps the best idea would be to provide some external API for this, that makes use of ObjectManager. Some sort of high-level wrapper around ObjectManager, a service proxy of sorts. 🤔

@sander-machado

sander-machado commented May 30, 2026

Copy link
Copy Markdown
Author

If ObjectManager exists try to use that

That would solve the problem at hand (properties cache) but user will still have a proxy to a non-existant interface+object and any method calls will fail and they've also no way to know the state of the properties cache.

Keeping that in mind, I'm starting to think that perhaps the best idea would be to provide some external API for this, that makes use of ObjectManager. Some sort of high-level wrapper around ObjectManager, a service proxy of sorts. 🤔

yeah I think the main underlying issue here actually is that you can receive an object from Proxy::new() that is actually invalid as well. Changing this would be a massive breaking API change, making new async and making sure the targeting service + interface + path exists. If we only add the functionality if ObjectManager exist, you won't solve the issue for people that use service that don't implement it though. For example my own use case would not be fixed so I would definitely prefer a solution that at least works without it, even if it's less good? I don't think ObjectManager is that wide spread yet?

But maybe having some higher level service proxy could be good idea regardless, it could then have a async method that returns the Proxy in question, that would not be an API change as well which is a plus. But existing users wouldn't get the fix for "free"

edit: new is already async, was thinking of something else and got confused sorry, maybe new should simply Error out if the backing service doesn't exist? and we create a new method that's basically new + wait till it exists?

@zeenix

zeenix commented May 31, 2026

Copy link
Copy Markdown
Contributor

Changing this would be a massive breaking API change, making new async and making sure the targeting service + interface + path exists.

That's not at all what I was proposing. It's not at all a catastrophe to to have proxies to in-existant interfaces (I believe other D-Bus APIs have the same "issue").

If we only add the functionality if ObjectManager exist, you won't solve the issue for people that use service that don't implement it though. For example my own use case would not be fixed so I would definitely prefer a solution that at least works without it, even if it's less good? I don't think ObjectManager is that wide spread yet?

AFAIK ObjectManager is high recommended when your objects and interfaces are dynamic since it's a very bad API to add and (especially) remove objects/interfaces w/o notiying any clients and therefore it is quite widespread. Us providing a better interface for this, would further encourage people to implement it. With zbus, it's super easy to implement it even.

But maybe having some higher level service proxy could be good idea regardless, it could then have a async method that returns the Proxy in question, that would not be an API change as well which is a plus. But existing users wouldn't get the fix for "free"

I think it'd be good to do both. In the proxy, we should keep it simple though and only care about service/name, and not complicate things with ObjectManager etc. ObjectManager would also be quite complicated and heavy to use correctly here. The interface doesn't have a DoesThisObjectExists method but rather you'll have to get a list of all the objects per proxy and there could be thousands of objects and interfaces in a single service. There is also the issue of race condition of objects changing while GetManagedObjects reply is being handled, that will need to be handled and proxy is already quite complicated.

This is why I think ObjectManager-based API should be an external umbrellla API for the whole service, so that it can scale and there isn't a lot of complication in the proxy.

@sander-machado

Copy link
Copy Markdown
Author

Changing this would be a massive breaking API change, making new async and making sure the targeting service + interface + path exists.

That's not at all what I was proposing. It's not at all a catastrophe to to have proxies to in-existant interfaces (I believe other D-Bus APIs have the same "issue").

Yeah in our C++ libdbus wrapper the "proxy" just gets created but because it doesn't use caching it kinda just works because it's basically just a bunch of match rules. We migrate one of those over to rust and that's when I stumbled on this behavior which is very initiative in zbus. If you disable caching does zbus work the same? There's a comment on the API that gives you the property changed stream that it doesn't work without caching enabled (never actually tested it).

If we only add the functionality if ObjectManager exist, you won't solve the issue for people that use service that don't implement it though. For example my own use case would not be fixed so I would definitely prefer a solution that at least works without it, even if it's less good? I don't think ObjectManager is that wide spread yet?

AFAIK ObjectManager is high recommended when your objects and interfaces are dynamic since it's a very bad API to add and (especially) remove objects/interfaces w/o notiying any clients and therefore it is quite widespread. Us providing a better interface for this, would further encourage people to implement it. With zbus, it's super easy to implement it even.

Yeah unfortunately for me we have our own version of ObjectManager API because it pre-dates ObjectManager so I would have to migrate. But if zbus actually works well it it, might be worth the investment. I would still like the basic case where it waits for the service to come online to be in the proxy though? Regardless of the ObjectManager support.

But maybe having some higher level service proxy could be good idea regardless, it could then have a async method that returns the Proxy in question, that would not be an API change as well which is a plus. But existing users wouldn't get the fix for "free"

I think it'd be good to do both. In the proxy, we should keep it simple though and only care about service/name, and not complicate things with ObjectManager etc. ObjectManager would also be quite complicated and heavy to use correctly here. The interface doesn't have a DoesThisObjectExists method but rather you'll have to get a list of all the objects per proxy and there could be thousands of objects and interfaces in a single service. There is also the issue of race condition of objects changing while GetManagedObjects reply is being handled, that will need to be handled and proxy is already quite complicated.

This is why I think ObjectManager-based API should be an external umbrellla API for the whole service, so that it can scale and there isn't a lot of complication in the proxy.

So as a suggestion, in this PR I see two ways I could implement the simple Proxy usecase.

  • idea1: any async request send to the proxy get's "queued" on some async init function that waits for the service to appear. This makes the code quite ergonomic to use but you won't notice if the service never comes online. For my personal use case this makes a lot of sense because I know that the target interface will always be there but might be confusing for other users?
  • idea2: property gets continue to throw but property changed stream will start producing values when the target service/name comes online instead of doing nothing for ever like now?

Am I following your reasoning here? For the the ObjectManager stuff that should probably be a seperate "epic" or "issue" whatever you wanna call it? I can probably dedicate some time to once it's clear how you want it implemented.

@zeenix

zeenix commented May 31, 2026

Copy link
Copy Markdown
Contributor

Am I following your reasoning here? For the the ObjectManager stuff that should probably be a seperate "epic" or "issue" whatever you wanna call it? I can probably dedicate some time to once it's clear how you want it implemented.

Yeah, idea2 seems good to me. We need to to also take into account service going down and up again (during which time the property values can change).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants