Sign in to follow this  
Waterlimon

Limited lifetime of returned objects issue

Recommended Posts

I have an Application class with a std::vector<Monitor> getMonitors() method.

Monitor is a wrapper around a GLFWMonitor*.

 

getMonitors calls GLFWmonitor** glfwGetMonitors(int* count) to get the pointers, then wraps them into Monitors and returns a vector of them.

 

I could use the monitor objects to display a list of monitors to the user to select from, to eventually pass a Monitor to my Window class if I want a fullscreen window on that monitor (I can use the GLFWmonitor* to get the monitor name, or to pass it to a window creation function, internally).

 

GLFW docs say that the GLFWMonitor* pointers can be invalidated when physical monitors are connected/disconnected.

 

 

 

Thus, the returned Monitor objects should be consumed immediately, not stored for later use. I assume "immediately" means "before I call some glfw method that causes it to update its list of monitors".

 

How do I correctly enforce this limited lifetime of Monitor objects (if at all)?

 

Should I deal with monitors in some other way than returning a vector of pointer wrappers? Maybe Monitor should contain value copies of all monitor data (instead of containing GLFWmonitor* and using that to get the data) and when I need the corresponding GLFWmonitor* I internally do some kind of reverse lookup using the monitor name for example (are monitor names unique?)

Share this post


Link to post
Share on other sites

I don't think monitor names are unique - for instance my two monitors have the same name except one has 21" at the end and the other has 22" - and in my experience this kind of reverse lookup stuff tends to be brittle and unreliable. In the docs I see that you can register a callback to be notified of monitor connection/disconnection, I think I would personally use this to display to the user the *available* monitors and use it to make sure you never hold onto a pointer to a disconnected monitor.

Share this post


Link to post
Share on other sites

How do I correctly enforce this limited lifetime of Monitor objects (if at all)?


Return the monitor's index or unique ID as part of your data, rather than a pointer. The index is semi-stable; so long as a monitor is not connected or disconnected, the order will not change (according to the GLFW docs). When monitors are reconfigured, just send out an event to interested parties letting them know about this.

Indices/IDs are often a better solution to pointers for several reasons. A big one is that they're not tied to memory lifetimes and are never intrinsically "unsafe" the same way an unowned or borrowed pointer can be; if a client of your API wrapper gets an index, monitors are reconfigured, and then the client reuses that stale index, the absolute worst thing that should happen (assuming you check index bounds before using them) is that the client ends up selecting the wrong monitor. Hardly the end of the world.

And IDs can even solve that problem if you're smart. If the ID contains both an index and a version tag, and monitor reconfiguration increments a version counter, then any attempt to use an ID with a stale version can return an error that can be handled or at least reported to the user.

The non-versioned variant might look something like:

struct Monitor {
  int width, height;
  int index;
};

vector<Monitor> GetMonitors() {
  vector<Monitor> result;

  int count;
  GLFWmonitor** monitors = glfwGetMonitors(&count);

  for (int i = 0; i != count; ++i)
  {
    Monitor tmp;
    glfwGetMonitorPhysicalSize(monitors[i], &tmp.width, &tmp.height);
    tmp.index = i;
    results.push_back(tmp);
  }

  return results;
}

GLFWmonitor* SelectMonitor(int index)
{
  int count;
  GLFWmonitor** monitors = glfwGetMonitors(&count);

  if (index < 0 || index >= count)
    return nullptr;

  return monitors[index];
}

void CreateWindow(blah, int monitorIndex)
{
  int count;
  GLFWmonitor** monitors = glfwGetMonitors(&count);
  GLFWmonitor* monitor =
    monitorIndex >= 0 && monitorIndex < count ?
    monitors[monitorIndex] :
    glfwGetPrimaryMonitor();

  GLFWwindow* window = glfwCreateWindow(blah, blah, "Blah", monitor, nullptr);
}
Nice and simple and safe, if not fool proof. You might return an error on the monitorIndex being out of range instead of selecting a default, you might use the versioning technique I mentioned, and you might make the index more type safe (my favorite C++11 trick here is to just use a strong enum, e.g. `enum class MonitorId : int { None = -1 };` with the appropriate casts in the appropriate places in your function internals (`int(monitor)` or `MonitorId(index)`, and also you can use MonitorId::None as your default value, and again just bounds check before using the ID as an index).

Share this post


Link to post
Share on other sites

I think ill keep the pointer inside Monitor class, with the following 2 changes:

  • Store monitor name/size as copies inside monitor, so pointer doesnt have to be used for that (keeps Monitor class simpler, no glfw specifics)
  • Before using the pointer, check that it exists inside the list of monitors provided by GLFW. This avoids the issue of getting the wrong monitor that might happen with indices.

I guess I could also name the getMonitors method "getCurrentMonitorsSnapshot" but Id rather not :P

Share this post


Link to post
Share on other sites


Before using the pointer, check that it exists inside the list of monitors provided by GLFW. This avoids the issue of getting the wrong monitor that might happen with indices.

Probably okay since the pointer is still on the list of valid pointers.

 

I suppose in theory it could be a problem. The library could be using a pool of monitor settings, a monitor was disconnected and another was connected, you ignored the notice, but the newly connected monitor happens to use the same pointer.

 

If you decide to keep a pointer around -- rather than quickly using it and discarding it when done -- you should register to be notified of changes that would invalidate your pointer. (See glfwSetMonitorCallback)  The moment that is called you should assume the pointer is invalid, which could potentially be a problem if you are using the pointer in some other thread to do work.

 

 

Sure the odds of it happening are moderately rare, most people don't disconnect monitors mid-game, but there are some people on laptops who pick their game up off their docking station, you wouldn't want the game to crash in that scenario, or power management scenarios that shut down monitors, or players turning off the monitor to save power, or ... 

Share this post


Link to post
Share on other sites

Providing a callback or event for when a monitor changes is good enough most of the time but there's always a window for race conditions even if you attempt to validate the monitor handle before you use it. All the native platform APIs I've worked with simply return an error code if you pass it an invalid monitor handle so there's no reason why your own code can't just pass on a similar error code when that happens. Since you're using GLFW then you don't have much choice other than to provide a thin wrapper around glfwMonitor pointers. I don't know how robust GLFW is but it would be surprising (and bad API design) if any GLFW function caused a crash because you provided it with an expired monitor handle.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this