On Tue, May 30, 2017 at 8:27 PM, Dan Williams <d...@redhat.com> wrote: >> @@ -429,13 +536,15 @@ mm_common_cinterion_disable_location_gathering >> (MMIfaceModemLocation *self, >> typedef enum { >> ENABLE_LOCATION_GATHERING_GPS_STEP_FIRST, >> ENABLE_LOCATION_GATHERING_GPS_STEP_SGPSS, >> + ENABLE_LOCATION_GATHERING_GPS_STEP_SGPSC_OUTPUT, >> + ENABLE_LOCATION_GATHERING_GPS_STEP_SGPSC_ANTENNA, >> + ENABLE_LOCATION_GATHERING_GPS_STEP_SGPSC_ENGINE, >> ENABLE_LOCATION_GATHERING_GPS_STEP_LAST, >> } EnableLocationGatheringGpsStep; >> >> typedef struct { >> MMModemLocationSource source; >> EnableLocationGatheringGpsStep gps_step; >> - gboolean sgpss_success; >> } EnableLocationGatheringContext; >> >> static void >> @@ -454,28 +563,35 @@ mm_common_cinterion_enable_location_gathering_finish >> (MMIfaceModemLocation *sel >> >> static void enable_location_gathering_context_gps_step (GTask *task); >> >> -static void >> -enable_sgpss_ready (MMBaseModem *self, >> - GAsyncResult *res, >> - GTask *task) >> +static gboolean >> +enable_location_gathering_context_gps_step_next_cb (GTask *task) >> { >> EnableLocationGatheringContext *ctx; >> - GError *error = NULL; >> >> ctx = (EnableLocationGatheringContext *) g_task_get_task_data (task); > > Is there a way we can check a GCancellable here? If the modem gets > removed while we're enabling GPS this might call back with a dead > object.
We've discussed this in IRC already and we'll go on with the patch as it is for now. The rationale for this is that the GTask already holds a full reference of the modem object, so a valid reference will be available until the whole async method finishes. If the modem goes away in the middle of the sequence, we'll just end up doing some dummy timeouts (retrying for Engine = 1) and then just fail the async method, see e.g. the following logs: ModemManager[4839]: <debug> [1496171739.271335] Setting up location sources: '3gpp-lac-ci, gps-nmea' ModemManager[4839]: <debug> [1496171739.271399] Location '3gpp-lac-ci' gathering is already enabled... ModemManager[4839]: <debug> [1496171739.271415] Location 'gps-raw' gathering is already disabled... ModemManager[4839]: <debug> [1496171739.271427] Location 'gps-unmanaged' gathering is already disabled... ModemManager[4839]: <debug> [1496171739.271443] Need to enable the following location sources: 'gps-nmea' ModemManager[4839]: <debug> [1496171739.271637] (ttyACM1) device open count is 2 (open) ModemManager[4839]: <debug> [1496171739.271694] (ttyACM1): --> 'AT^SGPSC="NMEA/Output","on"<CR>' ModemManager[4839]: <debug> [1496171739.331314] (ttyACM1): <-- '<CR><LF>^SGPSC: "Nmea/Output","on"<CR><LF><CR><LF><CR><LF>OK<CR><LF>' ModemManager[4839]: <debug> [1496171739.331410] (ttyACM1) device open count is 1 (close) ModemManager[4839]: <debug> [1496171739.431720] (ttyACM1) device open count is 2 (open) ModemManager[4839]: <debug> [1496171739.431815] (ttyACM1): --> 'AT^SGPSC="Power/Antenna","on"<CR>' ModemManager[4839]: <debug> [1496171739.495599] (ttyACM1): <-- '<CR><LF>^SGPSC: "Power/Antenna","on"<CR><LF><CR><LF>OK<CR><LF>' ModemManager[4839]: <debug> [1496171739.495722] (ttyACM1) device open count is 1 (close) ModemManager[4839]: <debug> [1496171740.133830] (ttyACM0) unexpected port hangup! ModemManager[4839]: <debug> [1496171740.133888] (ttyACM0) forced to close port ModemManager[4839]: <debug> [1496171740.133918] (ttyACM0) device open count is 0 (close) ModemManager[4839]: <debug> [1496171740.133939] (ttyACM0) closing serial port... ModemManager[4839]: <debug> [1496171740.165232] (ttyACM0) serial port closed ModemManager[4839]: <debug> [1496171740.167708] (ttyACM1) unexpected port hangup! ModemManager[4839]: <debug> [1496171740.167770] (ttyACM1) forced to close port ModemManager[4839]: <debug> [1496171740.167791] (ttyACM1) device open count is 0 (close) ModemManager[4839]: <debug> [1496171740.167811] (ttyACM1) closing serial port... ModemManager[4839]: <debug> [1496171740.188124] (ttyACM1) serial port closed ModemManager[4839]: <debug> [1496171740.188506] Removing device 'USB3' ModemManager[4839]: <debug> [1496171740.188722] [device USB3] unexported modem from path '/org/freedesktop/ModemManager1/Modem/0' ModemManager[4839]: <debug> [1496171740.189045] (ttyACM2) forced to close port ModemManager[4839]: <debug> [1496171741.497337] GPS Engine setup failed (1/3) ModemManager[4839]: <debug> [1496171743.499526] GPS Engine setup failed (2/3) ModemManager[4839]: <debug> [1496171745.500241] GPS Engine setup failed (3/3) ModemManager[4839]: <debug> [1496171745.500593] Modem (Cinterion) 'USB3' completely disposed The last lines show how the GPS Engine setup retries are happening every 2s, until the last 3/3 where the async task finishes, the GTask gets disposed, and along with the GTask the last modem object reference. In order to avoid the retries when we know that the modem is already gone (something that also happens in other logic bits doing retries, like the unlock check retries), I'll try to revive the idea of having a "device wide cancellable" that we can attach to every async operation (e.g. even to the GTask themselves), so that we cancel that cancellable when the modem is invalid and any async operation that may be ongoing can also error out as Cancelled, see e.g. these old never-merged commits: https://cgit.freedesktop.org/ModemManager/ModemManager/commit/?h=aleksander/cancellations&id=7bbc6dbbf6b79b7605a506257a2a7195692d91a5 https://cgit.freedesktop.org/ModemManager/ModemManager/commit/?h=aleksander/cancellations&id=d34008221a59f2402fa1695a07a1535c965eaee2 https://cgit.freedesktop.org/ModemManager/ModemManager/commit/?h=aleksander/cancellations&id=89d5291af5c2362453dcf4759258ce03d508cfd1 -- Aleksander https://aleksander.es _______________________________________________ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel