Giter Club home page Giter Club logo

Comments (6)

LouisCharlesC avatar LouisCharlesC commented on August 16, 2024

Hey there, I'm glad you like the library!

I guess I see the problem. I also guess I like typing even less then you, because the first solution that comes to my mind is to wrap your devices into a std::optional. This would let you .reset() it whenever you like, effectively unlocking the mutex and throwing an exception if you try to access the value (given you use the throwing interface of std::optional).

See, access objects do not really do anything on their own. This is by design because it allows them to work why any kind of lock (having no expectation makes things very simple!). The .release() method you talk about couldn't work with a std::lock_guard, for instance, since it is impossible to have a std::lock_guard release its mutex short of destructing it. Or did I misinterpret the suggestion ?

Be aware though that you can use safe with std::unique_lock which does have an .unlock() method. Now sadly because of what I just said above, it is impossible for the access object to intercept the call to .unlock() and know that the value object should not be accessed anymore. So its not the safest solution, but it qualifies as a low typing solution.

Now, if you own the code and can change the way connect() works, you probably have other options.

Does any of this seem to solve your problem ?

from safe.

Amomum avatar Amomum commented on August 16, 2024

Hm... I forgot about the optional and it seemed to be the good solution indeed, however for some reason it doesn't compile in the most simple (and therefore, desired) form with CTAD:

    safe::Safe<int> test;

    auto t = std::optional{ test.readAccess() };

nor with a wordy:

std::optional< safe::ReadAccess< safe::Safe<int> > > t = test.readAccess();

They produce different but similarly bizarre errors; first one gives a lot of template errors that I can't comprehend quickly... sorry, I'm not that good with template wizardry :(

Can you please elaborate on how to use optional with an access object?


The .release() method you talk about couldn't work with a std::lock_guard, for instance, since it is impossible to have a std::lock_guard release its mutex short of destructing it. Or did I misinterpret the suggestion ?

No, you interpreted me correctly. Hm.
Well, you could use placement new (or optional :) to construct lock_guard and destruct it by explicit call to destructor or resetting the optional..

from safe.

LouisCharlesC avatar LouisCharlesC commented on August 16, 2024

Argh, I guess the "non-movable, non-copyable" nature of std::lock_guard strikes again. The compiler does not let you do what you want because you construct an Access object and then try to move it inside the std::optional. Access objects are by default non-movable and non-copyable because they by default contain a std::lock_guard.

Try with a std::unique_lock to confirm the problem: auto t = std::optional{test.readAccess<std::unique_lock>()};. It compiles. But if we want a std::lock_guard rather than a std::unique_lock, we must construct the Access object in place in the std::optional. Indeed, std::optional<safe::ReadAccess<safe::Safe<int>>> t2{std::in_place, test}; compiles. I never tried combining safe and std::optional before. Sorry it is a bit more messy than expected.

Now, I could do all that inside the Access objects and offer a release() method and I guess it could have some value in that it would tie the release of the mutex with the "release" of the value object pointed to by the Access object. But it would also totally allow one to by-pass the guarantees of std::lock_guard which are: if the lock lives, the mutex is locked. I want that guarantee in a default Access object: if the Accessobject lives, the mutex is locked and the value is accessible.

I would argue that your code seems to not be very "scopy". I prefer to deal with mutexes with clear scoped variables, when I can. In your case, it could mean to have the connect() method work with an already locked mutex. Maybe it could accept an Access object as argument, No need for a release() method, then. Maybe it is impossible in your case, but if it is so, you can always to resort to the std::optional we just discovered. Sorry for the free jab to your code ;)

from safe.

Amomum avatar Amomum commented on August 16, 2024

I want that guarantee in a default Access object: if the Accessobject lives, the mutex is locked and the value is accessible.

Fair enough, can't argue with that one. I'm also not a big fan of objects that can exist in a invalid state without being explicitly optional-like, however in C++ a lot of objects behave like that.. I guess we should not increase their numbers :)

I would argue that your code seems to not be very "scopy".

That is also a fair point. I've came up with a more scopy version so I guess I will stick to this style:

auto shouldConnect = [&] -> bool {
  auto devices = devices_.readAcces();

  auto dev = devices->find("somehow");
  if( dev == devices->end() ) {
    return true;
  }

  if( dev->connected() == false ) {
    return true;
  }

  return false;
}();

if( shouldConnect ) {
  connect("somehow");
}

Indeed, std::optional<safe::ReadAccess<safe::Safe>> t2{std::in_place, test}; compiles. I never tried combining safe and std::optional before. Sorry it is a bit more messy than expected.

I guess this could be hidden inside the library in something like getOptionalReadAccess but.. meh. Not sure if it's worth it.

from safe.

LouisCharlesC avatar LouisCharlesC commented on August 16, 2024

I guess this could be hidden inside the library in something like getOptionalReadAccess but.. meh. Not sure if it's worth it.

Agreed. I'm a "less is more" nazy. Others have tried to make me add stuff to the library and failed. If the point ever comes up again, I guess I'll reconsider.

I've came up with a more scopy version so I guess I will stick to this style

That looks much nicer :)

Should we consider this issue closed ?

from safe.

Amomum avatar Amomum commented on August 16, 2024

Others have tried to make me add stuff to the library and failed.

😅

Should we consider this issue closed ?

Yes! Thank you for your time!

from safe.

Related Issues (17)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.