Skip to content

Commit

Permalink
systemd: Don't open and close tuned DBus connections asynchronously
Browse files Browse the repository at this point in the history 
The TunedPerformanceProfile component was keeping a DBus connection
open to tuned, and was closing and reopening it whenever something
might have changed that requires it.

This happened asynchronously with the rest of the code that was using
this DBus connection, and would frequently cause that code to fail
with "disconnected" errors.

Also, the TunedDialog was itself causing changes that require a new
DBus connection (namely starting the tuned.service), but was always
working with the one that was originally passed into it via its
properties.

Let's straighten this all out:

 - Polling for the button state will open its own dedicated DBUs
   connection and close it when polling is done.  There is no need to
   keep a DBus connection open for the whole Cockpit session.

 - The tuned.service is started when the dialog is opened and a DBus
   connection is opened explicitly for the dialog after that and
   closed when the dialog is closed.
  • Loading branch information
@mvollmer
mvollmer committed Oct 22, 2024
1 parent 5b049f5 commit c94b7e5
Showing 1 changed file with 91 additions and 89 deletions.
180 changes: 91 additions & 89 deletions pkg/systemd/overview-cards/tuned-dialog.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/

import cockpit from "cockpit";
import React, { useState, useEffect, useCallback } from 'react';
import React, { useState, useRef } from 'react';

import { Button } from "@patternfly/react-core/dist/esm/components/Button/index.js";
import { Modal } from "@patternfly/react-core/dist/esm/components/Modal/index.js";
Expand All @@ -31,84 +31,82 @@ import { EmptyStatePanel } from 'cockpit-components-empty-state.jsx';
import { ModalError } from 'cockpit-components-inline-notification.jsx';
import { ProfilesMenuDialogBody } from './profiles-menu-dialog-body.jsx';
import { superuser } from 'superuser';
import { useObject, useEvent } from "hooks";
import { useEvent, useInit } from "hooks";
import { useDialogs } from "dialogs.jsx";

const _ = cockpit.gettext;

function poll_tuned_state(tuned, tunedService) {
return Promise.all([
tuned.call('/Tuned', 'com.redhat.tuned.control', 'is_running', []),
tuned.call('/Tuned', 'com.redhat.tuned.control', 'active_profile', []),
tuned.call('/Tuned', 'com.redhat.tuned.control', 'recommend_profile', [])
])
.then(([[is_running], [active_result], [recommended]]) => {
const active = is_running ? active_result : "none";
return ({ state: "running", active, recommended });
})
.catch((ex) => {
if (!tunedService.exists)
return ({ state: "not-installed" });
else if (tunedService.state != "running")
return ({ state: "not-running" });
else
return Promise.reject(ex);
});
}

export const TunedPerformanceProfile = () => {
const Dialogs = useDialogs();
const [btnText, setBtnText] = useState();
const [state, setState] = useState();
const [status, setStatus] = useState();
const oldServiceState = useRef(null);

const tunedService = useInit(() => service.proxy("tuned.service"));

async function update() {
const tuned = cockpit.dbus("com.redhat.tuned", { superuser: "try" });
try {
const { state, active, recommended } = await poll_tuned_state(tuned, tunedService);
let status;

if (state == "not-installed")
status = _("Tuned is not available");
else if (state == "not-running")
status = _("Tuned is not running");
else if (active == "none")
status = _("Tuned is off");
else if (active == recommended)
status = _("This system is using the recommended profile");
else
status = _("This system is using a custom profile");
setBtnText(state == "running" ? active : _("none"));
setState(state);
setStatus(status);
} catch (ex) {
console.warn("failed to poll tuned", ex);

setBtnText("error");
setStatus(_("Communication with tuned has failed"));
}
tuned.close();
}

useEvent(superuser, "reconnect", update);
useEvent(tunedService, "changed", () => {
// We get a flood of "changed" events sometimes without the
// state actually changing. So let's protect against that.
if (oldServiceState.current != tunedService.state) {
oldServiceState.current = tunedService.state;
update();
}
});

const tunedService = useObject(() => service.proxy("tuned.service"),
null,
[]);
const tuned = useObject(() => cockpit.dbus("com.redhat.tuned", { superuser: "try" }),
obj => obj.close(),
[superuser.allowed, tunedService.state]);

const poll = useCallback(() => {
return Promise.all([
tuned.call('/Tuned', 'com.redhat.tuned.control', 'is_running', []),
tuned.call('/Tuned', 'com.redhat.tuned.control', 'active_profile', []),
tuned.call('/Tuned', 'com.redhat.tuned.control', 'recommend_profile', [])
])
.then(([[is_running], [active_result], [recommended]]) => {
const active = is_running ? active_result : "none";
return ({ state: "running", active, recommended });
})
.catch((ex) => {
if (!tunedService.exists)
return ({ state: "not-installed" });
else if (tunedService.state != "running")
return ({ state: "not-running" });
else
return Promise.reject(ex);
});
}, [tunedService, tuned]);

const updateButton = useCallback(() => {
return poll()
.then(res => {
const { state, active, recommended } = res;
let status;

if (state == "not-installed")
status = _("Tuned is not available");
else if (state == "not-running")
status = _("Tuned is not running");
else if (active == "none")
status = _("Tuned is off");
else if (active == recommended)
status = _("This system is using the recommended profile");
else
status = _("This system is using a custom profile");

setBtnText(state == "running" ? active : _("none"));
setState(state);
setStatus(status);
})
.catch((ex) => {
console.warn("failed to poll tuned", ex);

setBtnText("error");
setStatus(_("Communication with tuned has failed"));
});
}, [poll, setBtnText, setState, setStatus]);

useEvent(superuser, "changed");
useEvent(tunedService, "changed", () => updateButton());

useEffect(() => {
updateButton();
}, [updateButton]);

const showDialog = () => {
Dialogs.show(<TunedDialog updateButton={updateButton}
poll={poll}
tunedDbus={tuned} tunedService={tunedService} />);
const showDialog = async () => {
await Dialogs.run(TunedDialog, { tunedService });
// Tuned does not send any change notifications...
await update();
};

return (
Expand All @@ -125,18 +123,22 @@ export const TunedPerformanceProfile = () => {
};

const TunedDialog = ({
updateButton,
poll,
tunedDbus,
tunedService,
dialogResult,
}) => {
const Dialogs = useDialogs();
const [tunedDbus, setTunedDbus] = useState(null);
const [activeProfile, setActiveProfile] = useState();
const [loading, setLoading] = useState(true);
const [error, setError] = useState();
const [profiles, setProfiles] = useState([]);
const [selected, setSelected] = useState();

function close() {
if (tunedDbus)
tunedDbus.close();
dialogResult.resolve();
}

/* Tuned doesn't implement the DBus.Properties interface, so
* we occasionally poll for what we need.
*
Expand Down Expand Up @@ -182,8 +184,6 @@ const TunedDialog = ({
console.warn("Failed to disable tuned profile:", results);
return Promise.reject(_("Failed to disable tuned profile"));
}

updateButton();
});
} else {
promise = tunedDbus.call('/Tuned', 'com.redhat.tuned.control', 'switch_profile', [selected])
Expand All @@ -193,18 +193,16 @@ const TunedDialog = ({
console.warn("Failed to switch profile:", results);
return Promise.reject(results[0][1] || _("Failed to switch profile"));
}

updateButton();
});
}

return promise
.then(setService)
.then(Dialogs.close)
.then(close)
.catch(setError);
};

useEffect(() => {
useInit(async () => {
const withInfo = (active, recommended, profiles) => {
const model = [];
profiles.forEach(p => {
Expand Down Expand Up @@ -240,7 +238,7 @@ const TunedDialog = ({
setSelected(active);
};

const withTuned = () => {
const withTuned = (tunedDbus) => {
const tunedProfiles = () => {
return tunedDbus.call('/Tuned', 'com.redhat.tuned.control', 'profiles2', [])
.then((result) => result[0])
Expand All @@ -250,7 +248,7 @@ const TunedDialog = ({
});
};

return poll()
return poll_tuned_state(tunedDbus, tunedService)
.then(res => {
const { state, active, recommended } = res;
if (state != "running") {
Expand All @@ -266,12 +264,16 @@ const TunedDialog = ({
.catch(setError);
};

tunedService.start()
.then(updateButton)
.then(withTuned)
.catch(setError)
.finally(() => setLoading(false));
}, [updateButton, poll, tunedService, tunedDbus]);
try {
await tunedService.start();
const tuned = cockpit.dbus("com.redhat.tuned", { superuser: "try" });
setTunedDbus(tuned);
await withTuned(tuned);
} catch (ex) {
setError(ex);
}
setLoading(false);
});

const help = (
<Popover
Expand Down Expand Up @@ -303,14 +305,14 @@ const TunedDialog = ({
className="ct-m-stretch-body"
isOpen
help={help}
onClose={Dialogs.close}
onClose={close}
title={_("Change performance profile")}
footer={
<>
<Button variant='primary' isDisabled={!selected} onClick={setProfile}>
{_("Change profile")}
</Button>
<Button variant='link' onClick={Dialogs.close}>
<Button variant='link' onClick={close}>
{_("Cancel")}
</Button>
</>
Expand Down

0 comments on commit c94b7e5

Please sign in to comment.