Re: Automatic Refresh for local Maildir's mail counts

--===============6828591058279850492==
Content-Type: multipart/signed; micalg=pgp-sha256;
	boundary="Sig_/5l5PbTJK7UJBd5vW9tid_te";
	protocol="application/pgp-signature"

--Sig_/5l5PbTJK7UJBd5vW9tid_te
Content-Type: multipart/mixed; boundary="MP_/YUVxtT8IMN_AycHXb++_Z4."

--MP_/YUVxtT8IMN_AycHXb++_Z4.
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline

[Sun, 1 Jan 2017 23:15:02 +0100] Linux-Fan <Ma_Sys.ma-S0/[email protected]> wrote:
> [Sat, 31 Dec 2016 12:27:38 -0500] Sam Varshavchik
> <[email protected]> wrote:
> > Linux-Fan writes:

[...]

> > > Is it possible/planned to add a means of updating the message
> > > counts from the folder hierarchy view automatically as well
> > > (similar to what I did upon receiving a SIGUSR1 for my
> > > proof-of-concept patch from the previous mail)?
> >=20
> > I'm open to such a feature; however this approach won't work.
> > Signals
>=20
> [...]
>=20
> > The only safe way to do this: in addition to installing a signal
> > handler, an internal pipe needs to be created, and put into
> > nonblocking mode. The only thing the signal handler does is write a
> > byte to the pipe, ignoring the results, and immediately terminating.
> >=20
> > Then, on the folder screen only, in addition to handling all the
> > regular events the read end of the pipe gets polled as well, and
> > when something is read, what your patch does would happen.
>=20
> OK. I am going to check this out when I find time to dig into the
> source code again :)

[...]

I have taken on the challenge and tried to create a (more) correct
patch which enables cone-0.92 to update all folder's message counts
upon receiving a SIGUSR1.

It is not using a pipe, but a C++11 std::atomic instead. If that cannot
be used, because Cone needs to compile without C++11 (I used
`./configure CPPFLAGS=3D-std=3Dc++11` to get it to work), I'd replace this
by a variable which is marked `volatile sig_atomic_t`.

What do you think? Does it make sense to put it that way or have I
misplaced the call to `processHierharcyRefresh()`? Is it better to use
`SA_RESTART` or not? I am not quite sure...
Also, in order to improve the folder refreshing mechanism, I have
implemented a `RefreshIterator` which uses `updateInfo` on all folder
entries. Have I used the API correctly?

> > Even with that, things may not work 100%. All the code needs to be
> > audited and verified that it'll correctly handle an interrupted
> > system call.
> >
> > Correct signal handling is hard. =20
>=20
> Yes it is :( Especially signal handling from within multithreaded
> applications is probably asking for trouble, so I understand that
> might not be the best solution here...

If signal handling is still too much of an issue with regard to the
overall application stability, might it make sense to replace my check
for `pendingUpdates.exchange(0) > 0` with a simple timer
(like if now - lastTimeOfCheck > NN seconds refresh)?

By the way: If you wonder about the `masysma` prefixes and annotations:
I will happily remove these if the patch has a chance of being
incorporated but for now I have added them to easily navigate to my
modifications within the code and make sure I do not confuse my
changes with what was already there...

Yours Sincerely and TIA
Linux-Fan

--=20
http://masysma.lima-city.de/

--MP_/YUVxtT8IMN_AycHXb++_Z4.
Content-Type: text/x-patch
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment; filename=patch_cone_3_safer_signals.diff

diff -Nau -x Makefile.in -x Makefile -x .deps -x '*.cache' -x buildversion.=
H -x '*.log' -x config.status -r cone-0.92/cone/cone.C cone-0.92_new/cone/c=
one.C
--- cone-0.92/cone/cone.C	2015-02-28 14:20:13.000000000 +0100
+++ cone-0.92_new/cone/cone.C	2017-02-01 21:19:07.334905194 +0100
@@ -58,6 +58,8 @@
 #include "macros.H"
 #include "configscreen.H"
=20
+#include "masysmarefreshsignal.H" // MASYSMA CHANGED
+
 #include <iostream>
=20
 using namespace std;
@@ -429,6 +431,7 @@
 			 myServer **selectedServer)
 {
 	CursesHierarchy hierarchy_screen( &myServer::hierarchy, mainScreen);
+	MasysmaRefreshSignal::setHierarchyPointer(&hierarchy_screen);
 =09
 	titleBar->setTitles(_("FOLDERS"), "");
=20
@@ -474,6 +477,8 @@
 		*selectedFolder=3Dhierarchy_screen.folderSelected;
 	if (selectedServer)
 		*selectedServer=3Dhierarchy_screen.serverSelected;
+
+	MasysmaRefreshSignal::setHierarchyPointer(NULL);
 }
=20
 void hierarchyScreen(void *dummy)
@@ -1185,6 +1190,7 @@
 	}
=20
 	signal(SIGPIPE, SIG_IGN);
+	MasysmaRefreshSignal::registerSignalHandler(); // MASYSMA CHANGED
 	macroPtr=3D &macroBuffer;
 	init(); // Common initialization between cone and leaf.
=20
diff -Nau -x Makefile.in -x Makefile -x .deps -x '*.cache' -x buildversion.=
H -x '*.log' -x config.status -r cone-0.92/cone/curseshierarchy.C cone-0.92=
_new/cone/curseshierarchy.C
--- cone-0.92/cone/curseshierarchy.C	2014-01-06 20:51:56.000000000 +0100
+++ cone-0.92_new/cone/curseshierarchy.C	2017-02-01 22:17:23.825889157 +0100
@@ -985,6 +985,28 @@
 	me->processDeletedFolder(parent, deleted, updatedFolder);
 }
=20
+// BEGIN MASYSMA CHANGED CODE
+bool CursesHierarchy::MasysmaRefreshIterator::visit(Hierarchy::Folder* f)
+{
+	if(f !=3D NULL && this->cs !=3D NULL)
+		f->updateInfo(this->cs, true);
+	return true;
+}
+
+bool CursesHierarchy::MasysmaRefreshIterator::visit(Hierarchy::Server* s)
+{
+	if(s !=3D NULL)
+		this->cs =3D s->getServer();
+	return true;
+}
+
+void CursesHierarchy::masysmaRefreshAllFolders()
+{
+	MasysmaRefreshIterator iterator;
+	getHierarchy()->root.prefixIterate(iterator);
+}
+// END MASYSMA CHANGED CODE
+
 ///////////
 bool CursesHierarchy::processKey(const Curses::Key &key)
 {
diff -Nau -x Makefile.in -x Makefile -x .deps -x '*.cache' -x buildversion.=
H -x '*.log' -x config.status -r cone-0.92/cone/curseshierarchy.H cone-0.92=
_new/cone/curseshierarchy.H
--- cone-0.92/cone/curseshierarchy.H	2014-01-06 20:51:56.000000000 +0100
+++ cone-0.92_new/cone/curseshierarchy.H	2017-02-01 22:10:43.324152755 +0100
@@ -61,6 +61,16 @@
=20
 	};
=20
+	// BEGIN MASYSMA CHANGED
+	class MasysmaRefreshIterator: public Hierarchy::EntryIterator {
+	private:
+		myServer* cs =3D NULL;
+	public:
+		bool visit(Hierarchy::Folder* f);
+		bool visit(Hierarchy::Server* s);
+	};
+	// END MASYSMA CHANGED
+
 public:
 	friend class DrawIterator;
=20
@@ -122,6 +132,11 @@
 public:
 	void processDeletedFolder(Hierarchy::Folder *, Hierarchy::Folder *,
 				  const mail::folder *);
+
+// BEGIN MASYSMA CHANGED CODE
+	void masysmaRefreshAllFolders();
+// END MASYSMA CHANGED CODE
+
 };
=20
 #endif
diff -Nau -x Makefile.in -x Makefile -x .deps -x '*.cache' -x buildversion.=
H -x '*.log' -x config.status -r cone-0.92/cone/Makefile.am cone-0.92_new/c=
one/Makefile.am
--- cone-0.92/cone/Makefile.am	2015-02-28 17:03:16.000000000 +0100
+++ cone-0.92_new/cone/Makefile.am	2017-02-01 20:16:17.879100122 +0100
@@ -81,7 +81,8 @@
 	spellchecker.H \
 	spellcheckerbase.C spellcheckerbase.H \
 	tags.C tags.H \
-	typeahead.C typeahead.H
+	typeahead.C typeahead.H \
+	masysmarefreshsignal.C masysmarefreshsignal.H # MASYSMA CHANGED
=20
 leaf_SOURCES=3D\
 	curseseditmessage.C \
diff -Nau -x Makefile.in -x Makefile -x .deps -x '*.cache' -x buildversion.=
H -x '*.log' -x config.status -r cone-0.92/cone/masysmarefreshsignal.C cone=
-0.92_new/cone/masysmarefreshsignal.C
--- cone-0.92/cone/masysmarefreshsignal.C	1970-01-01 01:00:00.000000000 +01=
00
+++ cone-0.92_new/cone/masysmarefreshsignal.C	2017-02-01 21:28:43.533566363=
 +0100
@@ -0,0 +1,50 @@
+// BEGIN MASYSMA CHANGED
+
+#include "masysmarefreshsignal.H"
+#include <atomic>
+#include <signal.h>
+
+CursesHierarchy* MasysmaRefreshSignal::masysmaHptr =3D NULL;
+
+// Signal handling related stuff is written in C instead of C++ as to keep
+// the function as simple as possible and leave as little potential for
+// interferences as possible.
+static std::atomic_int pendingUpdates(0);
+static void masysmaReceiveRefreshSignal(int arg0)
+{
+	pendingUpdates++;
+}
+
+void MasysmaRefreshSignal::registerSignalHandler()
+{
+	struct sigaction act;
+	struct sigaction oact; // original action, ignored for now...
+	act.sa_handler =3D &masysmaReceiveRefreshSignal;
+	sigemptyset(&act.sa_mask);
+	act.sa_flags =3D
+		// Restart running calls instead of returning EINTR.
+		// TODO: Is this behaviour wanted?
+		SA_RESTART |
+		// Simple signal handler function form
+		SA_SIGINFO ;
+	sigaction(SIGUSR1, &act, &oact);
+}
+
+void MasysmaRefreshSignal::setHierarchyPointer(CursesHierarchy* hptr)
+{
+	MasysmaRefreshSignal::masysmaHptr =3D hptr;
+}
+
+void MasysmaRefreshSignal::processHierarchyRefresh()
+{
+	// If the previous value before setting pendingUpdates to 0 was > 0
+	// and we are on the correct screen, we register a refresh.
+	// This means a signal received while not on the hierarchy screen is
+	// dropped. An alternative sematics enabling refreshing after returning
+	// to the hierarchy screen may be achieved by changing the order of
+	// operands for &&.
+	if(pendingUpdates.exchange(0) > 0 && masysmaHptr !=3D NULL)
+		MasysmaRefreshSignal::masysmaHptr->masysmaRefreshAllFolders();
+}
+
+// END MASYSMA CHANGED
diff -Nau -x Makefile.in -x Makefile -x .deps -x '*.cache' -x buildversion.=
H -x '*.log' -x config.status -r cone-0.92/cone/masysmarefreshsignal.H cone=
-0.92_new/cone/masysmarefreshsignal.H
--- cone-0.92/cone/masysmarefreshsignal.H	1970-01-01 01:00:00.000000000 +01=
00
+++ cone-0.92_new/cone/masysmarefreshsignal.H	2017-02-01 21:23:23.114216608=
 +0100
@@ -0,0 +1,14 @@
+#pragma once
+// BEGIN MASYSMA CHANGED
+#include "curseshierarchy.H"
+
+class MasysmaRefreshSignal {
+private:
+	static CursesHierarchy* masysmaHptr;
+public:
+	static void registerSignalHandler();
+	static void setHierarchyPointer(CursesHierarchy* hptr);
+	static void processHierarchyRefresh();
+};
+
+// END MASYSMA CHANGED
diff -Nau -x Makefile.in -x Makefile -x .deps -x '*.cache' -x buildversion.=
H -x '*.log' -x config.status -r cone-0.92/cone/myserver.C cone-0.92_new/co=
ne/myserver.C
--- cone-0.92/cone/myserver.C	2014-01-06 20:51:56.000000000 +0100
+++ cone-0.92_new/cone/myserver.C	2017-02-01 21:18:47.854965089 +0100
@@ -18,6 +18,7 @@
 #include "curses/cursesstatusbar.H"
 #include "libmail/misc.H"
 #include "gettext.H"
+#include "masysmarefreshsignal.H" // MASYSMA CHANGED
 #include <time.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -281,6 +282,8 @@
 				break;
 			}
 		}
+
+		MasysmaRefreshSignal::processHierarchyRefresh(); // MASYSMA CHANGED
 	}
=20
 	statusBar->notbusy();

--MP_/YUVxtT8IMN_AycHXb++_Z4.--

--Sig_/5l5PbTJK7UJBd5vW9tid_te
Content-Type: application/pgp-signature
Content-Description: OpenPGP digital signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJYklYxAAoJEPhJcO2yngi1z0UQAKXnJSCsQFLiYAb+AWWuw/It
8q+rMAS+Ypq77B2vHhH8AD6DjYhc9sXVwaTFiD+ELJLf9ssOvDR+cgdpI2GCz836
T/yPM7jLNjiAEXWu4YmZl9xhO2iD8WRotVx+TqhH8TfmTu68vplfRFzle/ruAJMV
Czrg4yUln4C3p5g7jROHa9llMJ7rIIanIPeDFYRFPLxSwa86NZGRqkqaLbku8ooQ
f3WbDkp72oW7Hj9Hjm2srD+h7onE41Mq63pTb0Oi+m1sI/Rpk4QNveaA7fBRAAvO
HFMbbWh63qvV0FRPROun46JBd8gC6HPizd1vO5qWf0NQWAAjx+n2VbSeIY3kMRzy
xBcOdnfdTQoXL8Ai8rUbRqV3czcdLV7AgLi+nQLKPS/cWhd6lPmFvpZxrZupAfBF
QmrrrjSG1U2W+O+71719l3mJymxDonZoQpzTZZRUuAHr6ORq+q0FE04/sfVZq6nA
DYfdpAmW8cyBArG8w96VKXNlilOB2593sJzfa4g+VpXh+UUF9CvkYk8BAgfaRnAo
f5ggUtsAVqpwqktFAnBojWIf6fo/dsNzLNOVEGv8GIrKAgq++vFmsG+JICNTMDJ0
u1gIFiVEkH986byjZIO8dFOOkjAMZ8NrZHbQNL/Qhb1UoF59ETV6AQREuBRdYF/+
ymDimry/yEq4GmP9WlcD
=SG8y
-----END PGP SIGNATURE-----

--Sig_/5l5PbTJK7UJBd5vW9tid_te--


--===============6828591058279850492==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
--===============6828591058279850492==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
Courier-cone mailing list
[email protected]
Unsubscribe: https://lists.sourceforge.net/lists/listinfo/courier-cone

--===============6828591058279850492==--