Savers: make every port_free and port_new path unconditional

Follow-up to 7aa4547e60. Four ports -- flux, plasma, solarwinds and
hyperspace -- still opened port_free with "if (!g_started) return;". That is
unreachable today, since the JNI only calls destroy after a successful
create, but unreachability is a property of the current call graph rather
than of the function, and it contradicts the rule the fix established: gl1
is given back on every path out, because whatever it holds belongs to an
EGL context that is about to die. All six are now unconditional.

Same argument one level up. port_new opened with "if (g_started) return 1",
which was defensible when gl1's state was whatever the last saver left, but
nativeInit now calls gl1_lost() first -- so gl1 is guaranteed DOWN on entry
and reporting success there would hand the caller a saver with no shim under
it. A stale run is torn down instead, and gl1_init always runs.

No behaviour change on any path reachable today. The point is that a saver
added later, or an upstream cleanup that returns early, fails in its own
saver instead of poisoning the next one.
This commit is contained in:
jpolo1224
2026-08-23 10:49:41 -04:00
parent fc5c24f8a3
commit 6451202102
6 changed files with 86 additions and 22 deletions
@@ -23,10 +23,16 @@ namespace { bool g_started = false; }
extern "C" {
void flux_port_free(); /* defined below; port_new tears down a stale run */
/* preset is 1..6, matching the saver's own DEFAULTS1..DEFAULTS6. */
int flux_port_new(int preset)
{
if (g_started) return 1;
/* NOT "return 1". g_started can only be set here if a previous run was never
* freed, and nativeInit has already called gl1_lost() -- so gl1 is DOWN, and
* reporting success would hand the caller a saver with no shim under it. Tear the
* stale run down and start clean. */
if (g_started) flux_port_free();
if (!gl1_init()) return 0;
if (preset < 1 || preset > 6) preset = 1;
@@ -65,11 +71,18 @@ void flux_port_draw()
void flux_port_free()
{
if (!g_started) return;
saver_flux::cleanUp();
if (g_started) {
saver_flux::cleanUp();
saver_flux::readyToDraw = 0;
g_started = false;
}
/* Unconditional, and NOT guarded by g_started. gl1_init() ran in port_new, and everything
* it holds -- the shader program, the vertex buffers -- belongs to the EGL context that is
* about to be destroyed. Leaving g.ready set with names from a dead context poisons the NEXT
* saver, because gl1_init() early-returns on g.ready. That the JNI currently only calls this
* after a successful create is a property of today's call graph, not of this function.
* gl1_shutdown() is idempotent. */
gl1_shutdown();
saver_flux::readyToDraw = 0;
g_started = false;
}
} /* extern "C" */
@@ -24,10 +24,16 @@ namespace { bool g_started = false; }
extern "C" {
void hyperspace_port_free(); /* defined below; port_new tears down a stale run */
int hyperspace_port_new(int preset)
{
(void) preset; /* No presets upstream; every knob was a registry value. */
if (g_started) return 1;
/* NOT "return 1". g_started can only be set here if a previous run was never
* freed, and nativeInit has already called gl1_lost() -- so gl1 is DOWN, and
* reporting success would hand the caller a saver with no shim under it. Tear the
* stale run down and start clean. */
if (g_started) hyperspace_port_free();
if (!gl1_init()) return 0;
saver_hyperspace::setDefaults();
@@ -59,11 +65,18 @@ void hyperspace_port_draw()
void hyperspace_port_free()
{
if (!g_started) return;
saver_hyperspace::cleanUp();
if (g_started) {
saver_hyperspace::cleanUp();
saver_hyperspace::readyToDraw = 0;
g_started = false;
}
/* Unconditional, and NOT guarded by g_started. gl1_init() ran in port_new, and everything
* it holds -- the shader program, the vertex buffers -- belongs to the EGL context that is
* about to be destroyed. Leaving g.ready set with names from a dead context poisons the NEXT
* saver, because gl1_init() early-returns on g.ready. That the JNI currently only calls this
* after a successful create is a property of today's call graph, not of this function.
* gl1_shutdown() is idempotent. */
gl1_shutdown();
saver_hyperspace::readyToDraw = 0;
g_started = false;
}
} /* extern "C" */
@@ -23,9 +23,15 @@ int g_preset = 1;
extern "C" {
void lattice_port_free(); /* defined below; port_new tears down a stale run */
int lattice_port_new(int preset)
{
if (g_started) return 1;
/* NOT "return 1". g_started can only be set here if a previous run was never
* freed, and nativeInit has already called gl1_lost() -- so gl1 is DOWN, and
* reporting success would hand the caller a saver with no shim under it. Tear the
* stale run down and start clean. */
if (g_started) lattice_port_free();
if (!gl1_init()) return 0;
g_preset = (preset >= 1 && preset <= 6) ? preset : 1;
@@ -16,9 +16,15 @@ namespace { bool g_started = false; }
extern "C" {
void plasma_port_free(); /* defined below; port_new tears down a stale run */
int plasma_port_new(int preset)
{
if (g_started) return 1;
/* NOT "return 1". g_started can only be set here if a previous run was never
* freed, and nativeInit has already called gl1_lost() -- so gl1 is DOWN, and
* reporting success would hand the caller a saver with no shim under it. Tear the
* stale run down and start clean. */
if (g_started) plasma_port_free();
if (!gl1_init()) return 0;
saver_plasma::setDefaults();
@@ -66,11 +72,18 @@ void plasma_port_draw()
void plasma_port_free()
{
if (!g_started) return;
saver_plasma::cleanUp();
if (g_started) {
saver_plasma::cleanUp();
saver_plasma::readyToDraw = 0;
g_started = false;
}
/* Unconditional, and NOT guarded by g_started. gl1_init() ran in port_new, and everything
* it holds -- the shader program, the vertex buffers -- belongs to the EGL context that is
* about to be destroyed. Leaving g.ready set with names from a dead context poisons the NEXT
* saver, because gl1_init() early-returns on g.ready. That the JNI currently only calls this
* after a successful create is a property of today's call graph, not of this function.
* gl1_shutdown() is idempotent. */
gl1_shutdown();
saver_plasma::readyToDraw = 0;
g_started = false;
}
}
@@ -26,10 +26,16 @@ bool g_started = false;
extern "C" {
void skyrocket_port_free(); /* defined below; port_new tears down a stale run */
int skyrocket_port_new(int preset)
{
(void) preset; /* No presets upstream; every knob was a registry value. */
if (g_started) return 1;
/* NOT "return 1". g_started can only be set here if a previous run was never
* freed, and nativeInit has already called gl1_lost() -- so gl1 is DOWN, and
* reporting success would hand the caller a saver with no shim under it. Tear the
* stale run down and start clean. */
if (g_started) skyrocket_port_free();
if (!gl1_init()) return 0;
saver_skyrocket::setDefaults();
@@ -15,11 +15,17 @@ namespace { bool g_started = false; }
extern "C" {
void solarwinds_port_free(); /* defined below; port_new tears down a stale run */
/* preset is 1..6 from the UI. Upstream's DEFAULTS1..DEFAULTS6 is a zero-based ENUM here, not
* the 1-based #defines Flux uses, so the UI value is shifted down. */
int solarwinds_port_new(int preset)
{
if (g_started) return 1;
/* NOT "return 1". g_started can only be set here if a previous run was never
* freed, and nativeInit has already called gl1_lost() -- so gl1 is DOWN, and
* reporting success would hand the caller a saver with no shim under it. Tear the
* stale run down and start clean. */
if (g_started) solarwinds_port_free();
if (!gl1_init()) return 0;
if (preset < 1 || preset > 6) preset = 1;
@@ -52,11 +58,18 @@ void solarwinds_port_draw()
void solarwinds_port_free()
{
if (!g_started) return;
saver_solarwinds::cleanUp();
if (g_started) {
saver_solarwinds::cleanUp();
saver_solarwinds::readyToDraw = 0;
g_started = false;
}
/* Unconditional, and NOT guarded by g_started. gl1_init() ran in port_new, and everything
* it holds -- the shader program, the vertex buffers -- belongs to the EGL context that is
* about to be destroyed. Leaving g.ready set with names from a dead context poisons the NEXT
* saver, because gl1_init() early-returns on g.ready. That the JNI currently only calls this
* after a successful create is a property of today's call graph, not of this function.
* gl1_shutdown() is idempotent. */
gl1_shutdown();
saver_solarwinds::readyToDraw = 0;
g_started = false;
}
}