Avoid cases where update data is refreshed on admin pageloads and ensure update notifications are always sent as configured

This doesn't need a setting. What we need is to ensure that there's no possible way for update.module to attempt to fetch new available update data via the requirements checking via update_help() on admin/*. If we have cached available update data, we can see if we're missing a security release, but if we don't have data, we should just give up there. This is intimately related to the problems at #220592: Core cache API breaks update.module: fetches data way too often, kills site performance, etc but this is a slightly different part of the bug, that will need a different kind of patch. Therefore, I think these should be handled in separate issues to make each patch easier to review and commit. At least, I hope it'll be easier this way. ;) If not, we can always roll the fix for this back into #220592 and mark this one duplicate after all...

I'm going to call this "won't fix" based on re-reading #209242: update.module reports bogus results when .info files are newer than the cached available update data in detail (where the code in update_requirements() that can trigger a re-fetch was added -- see also #200028: Add db caching to update_get_projects() and update_calculate_project_data()). Once #220592: Core cache API breaks update.module: fetches data way too often, kills site performance, etc lands, these caches will actually work, and we're not going to be having the massive problems people are currently seeing. We really can't afford to *never* invalidate the data during hook_requirements(). However, once #220592 is in, we'll almost never do it except during the edge cases that #209242 was designed to fix...

Therefore, there's nothing to fix here...

Given that there have been multiple issues floating around related to update_refresh() running too often or failing, etc., I want to be sure not to open a new one. From what I can see, this is is currently the best issue to work with.

I've done my best to sort through the discussion that lead up to #220292 being committed (which seems to have addressed the majority of issues related to the frequency of update_refresh() running, etc.). I've also had a look at #209242 and generally follow the logic that was added to check the ctime value of .info files to ensure update_refresh() gets triggered when cached data may be stale. The problems I've observed recently seem to be related to the combination of these two fixes that can play out in certain circumstances. Namely it's still possible to have update_refresh() running somewhat frequently on a /admin page load (with notable wait times), and it also seems that update notifications may sometimes be missed.

Here is what seems to lead to these problems:

1. Admins have the "Check for Update" frequency set rather high (7 days).
2. Within this frequency, changes happen to a module that leads to an update to a .info file (a simple module update or any kind of file system adjustment that can lead to a change in a .info inode).
3. Caches expire, specifically any php static cache info related to ctime data along with the project cache data.
4. An admin visits an /admin page within the frequency defined in #1 and after #2 and #3 take place.

What seems to happen under these conditions is that visiting the admin page will bypass the cache mechanisms and force an update_refresh(). This then leads to the specific issues I alluded to above:

1. The admin page will not load until after update_refresh() finishes, which can be some time. This is a minor inconvenience for admins as this would not happen all that often, but it's still a problem as one would not want this check to happen during an actual page load.
2. The variable update_last_check will be reset to the current time, but _update_cron_notify() will not be run. So if this situation is encountered at least once every 7 days, (which I suppose is not too unlikely), _update_cron_notify() will never really be fired upon a cron run, and any update notifications will not go out.

It would seem that the first problem could be addressed by ensuring that the project .info ctime check (that's part of update_get_available()) also happens on cron. This would make it likely that cron would trigger the update_refresh() in the "background" before the update is requested due to a visit to an /admin page.

I suppose that a solution to the second problem could be to introduce a new variable (e.g. update_last_notify) that would be used to track the frequency that _update_cron_notify() runs, and update that separately from update_last_check. This way, the "Check for Update" configuration could be maintained regardless of the frequency of update_refresh(). An alternative to this could be to allow _update_cron_notify() to be triggered whenever update_refresh() runs. This could lead to notifications going out more often than desired, but at least they would not be missed.

I just want to get my preliminary analysis out there (hopefully in the correct issues thread) and see if there is something to this. I am more than happy to personally explore patches, but want to first see if I'm correctly isolating the problem I have been witnessing.

I'll take a stab at an initial patch here. I'm hesitant to introduce a new variable, but the more I think about it the more it seems like the best solution. We need a way to track when update_refresh() runs (update_last_check) in order for the project cache timing to work, but this frequency can't effectively be linked to the update notification functionality. So it just seems cleaner to have a separate timestamp variable for each.

The attached patch introduces this new variable (update_last_notify) and ties it to the update notification cycle, while leaving the update_last_check variable with the sole responsibility of tracking when update_refresh() runs. I've also added some logic to run update_get_available() upon each cron run to ensure the project cache is updated in the background, thus avoiding the remaining cases where it could still be fired during an /admin pageload.

I did at bit more research on this and discovered the following:

1. It looks like Drupal 7 triggers update_get_available() upon update_cron(), which is one of the changes introduced in my patch. This would seem to support the idea that this is a good way to ensure that update data is not refreshed on admin pageloads.
2. I found a similar issue being discussed for Drupal 7/8 at #1263040: No notifications sent when updates are available (followup and tests). There the pending solution also introduces a new variable that tracks the update notification interval separately from the update check interval. This would seem to support this strategy (also a change in my patch) for ensuring notifications are sent at the correct interval.

So I think I may at least be on the correct track with my approach. With that I went ahead and made some small adjustments to the patch (made sure I'm using the same update variable name being discussed in the Drupal 7/8 approach, more consistent comment terminology, etc.).

I'm hesitant to mark this issue as a duplicate of #1263040: No notifications sent when updates are available (followup and tests) for a couple reasons:

1. The approach being discussed in #1263040 only addresses one of the problems being explored in this issue (the problem where update data is refreshed on admin pageloads is unique to D6).
2. My guess is that #1263040 (marked for D8) won't receive much review attention by those using D6

Marking this as "major" because it relates to a potential failure in the update notification system that some may depend on for security update information.

@rjacobs, looks like you made a substantial investigation. Can you please describe code paths which lead to downloading the update information and sending email notifications? The logic seems quite convoluted for me and, afaik, undocumented. I guess it's the primary reason of the current sad situation.

Hi akamaus. Do you mean provide some added core code documentation in patch form, or provide additional elaboration in this queue about the original vs. patched logic introduced in #7?