Make sure ABP engine closes gracefully.

[trev]

Background

See #58.

There were reports of a corrupt (not full) patterns.ini file a few times, which is pretty bad since it doesn't get updated until the next update interval and so user gets unpredictable blocking behavior.

What to change

Make sure that whenever we have received a signal to exit from AdblockPlusEngine we are not interrupting any I/O operations.
Also we need to make sure that the engine does not shut down not having replied anything to the browser.

Essentially this TODO has to be addressed: ​https://github.com/abby-sergz/adblockplusie/commit/6caf0e7281c3c4e5955cc6a63e3adf85bdd523a5#diff-cb4693d37bb412be109c3776d5c985d8R423

[philll]

The assigned state will be dropped by #403

[oleksandr]

This means that we need to make sure we are not interrupting any I/O operations with closing of AdblockPlusEngine.

[oleksandr]

This means that we need to make sure we are not interrupting any I/O operations with closing of AdblockPlusEngine.

[trev]

While I am supposedly the reporter of this, I don't really understand what it is about. What's the problem with interrupting downloads on shut down? We do it on every other platform, the engine will simply restart the download when it is started later.

[oleksandr]

Modified the ticket description to be more precise.

[philll]

@trev: looks like this is one of the issues that I migrated from trello in your name, as the trello card was from you.

[trev]

@philll: It actually sounds like one of my review comments but the context is missing.

[eric@adblockplus.org]

I think I was the one that first raised this issue, because it arose originally as an installation issue.

When IE shuts down, it shuts down the BHO as well. That part works just fine. The problem is that the engine does not shut down when IE does. In all cases, this is a defect; if IE isn't running, then no part of ABP should be either. Ordinarily, though, it's in the class of minor defects; it just sits there taking up a process entry and some swap space and might occasionally wake up for updates. You can kill the process manually in this state with no adverse affects. So this is the first aspect of the issue: to shut down the engine when IE terminates.

During installation, though, it causes more serious problems. To avoid having Windows Installer schedule a reboot, it's necessary to have the both BHO DLL and the engine closed at the time of installation. (All that would be needed would be to reboot IE itself rather than the whole system, but Windows Installer has no way of specifying such a restricted dependency; it's the whole system or nothing.) The custom action inside the installer sends WM_ENDSESSION to windows of both IE and the engine process, and if those don't work WM_CLOSE. Now ordinarily IE will shut down just fine and take the plugin with it. The engine, however, doesn't have a way of separately honoring this window message, because the only time it has a window is for the update dialog box. So the second aspect of this issue is to have a hidden window associated with the engine that will honor WM_ENDSESSION.

The third aspect is the question of pending downloads at exit. During installation, it would be better to terminate everything immediately to reduce installation time. This would include interrupting any pending download at the least. (Perhaps we also want to enter a "terminating" state and short-circuit any pipe queries.) In ordinary use, we could complete the download normally in the background and then (silently) exit. Minimally, though, we need to do either one of these and ensure that the filter lists are in a consistent state on disk. And that's the third part of the issue, to ensure that no matter what happens, we leave things in a consistent state.

[sergz]

The problem is that the engine does not shut down when IE does.

The Engine should not be running when there are no clients and I think it is how it works now, the number of clients is zero => exit

to shut down the engine when IE terminates.

Not sure about it, I would like to avoid sticking to the IE, we can have other clients and it's not a problem for the Engine, it should not know anything about the clients.

All that would be needed would be to reboot IE itself rather than the whole system, but Windows Installer has no way of specifying such a restricted dependency; it's the whole system or nothing.

Could restart manager ​http://msdn.microsoft.com/en-us/library/windows/desktop/cc948910(v=vs.85).aspx help here?

During installation, it would be better to terminate everything immediately to reduce installation time. This would include interrupting any pending download at the least.

I would like to make the downloader interruptable. It's very easy, just pass delegate like bool IsCanceling() to the downloader/updater and call it when it's safe to stop the downloading process from the downloader/updater. I think it does not cause a big delay and users won't mind about it, especially if we can provide them with some progress indicator.

The value returned by the delegate can be based on the current number of active clients as well as we can have a hidden window (only for message loop) to be notified by the installer.

Minimally, though, we need to do either one of these and ensure that the filter lists are in a consistent state on disk. And that's the third part of the issue, to ensure that no matter what happens, we leave things in a consistent state.

It should be checked when the engine is starting, and fixed by the enegine if it's necessary, for example, by downloading a fresh list.

[Elegantitservices]

spam