Re: Automatic Refresh for local Maildir's mail counts

--===============7466312358514830708==
Content-Type: multipart/signed; micalg=pgp-sha256;
	boundary="Sig_/JycL8HZ4S.LvDIAVHLb9rIY";
	protocol="application/pgp-signature"

--Sig_/JycL8HZ4S.LvDIAVHLb9rIY
Content-Type: multipart/mixed; boundary="MP_/.NSW=EygP_k.2PxHI_y2FjO"

--MP_/.NSW=EygP_k.2PxHI_y2FjO
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline

[Wed, 01 Feb 2017 21:44:39 -0500] Sam Varshavchik
<[email protected]> wrote:
> Linux-Fan writes:
>=20
> > 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`.
>=20
> Cone is still compiled on relatively old platforms. C++11 is not a
> viable option right now; but probably in a little while this would be
> an option.

OK. I have changed the code to no longer rely on an atomic primitive. I
have implemented this by maintaining two counters where the first one
counts the number of signals received and the second one counts the
number of signals processed. This way, there is no potential for losing
a signal as there are no concurrent writes to the variables. (Well,
theoretically signals may still get lost: If exactly 2^32 signals are
received before Cone runs the function to detect pending updates, there
will not be an update, because advancing the counter 2^32 times puts it
back to it's initial position)

> I do not see a good reason not to use an internal pipe, in
> non-blocking mode, and with O_CLOEXEC set.

I have not tried to do it with pipes because I find them more complex:
Pipes need to be opened explicitely and I have not found a definite
answer about the question whether POSIX specifies writing to a pipe
from inside a signal handler to behave well-defined.

Also, I am not sure (for the pipe-idea) what happens if for any reason,
a lot of signals are received: My current solution will just keep
updating the messages all the times it is polled by the event loop.
Whenever a flood of signals stops, it will also do at most one update
after that. If I were to implement a pipe to detect signals, the pipe
would be filled (completely) whenever a flood of signals arises. On the
other end, my implementation should attempt to read as many entries as
possible (at once) from the pipe in order not to keep updating multiple
times even though the time the update was necessary is long in the
past. It might introduce a potential for nontermination if one end of
the pipe is continuously populated by signals while the other end is
continuously read (but never empty as for a steady stream of arriving
signals)...

All in all, I find pipes harder to reason about and thus went for the
easiest solution: Initially one atomic integer and now: two integers.

> sig_atomic_t has been around only since C99; but I suppose at this
> time requiring C99 would be acceptable.

Additionally, it should be available on every POSIX-compatible system.
This time, no additional options to `configure` were needed.

[...]

> No big rush here. You can take your time and test your changes
> thoroughly. In general, this looks fine but I have not had a closer
> look; but I will consider it. A few procedural things will need to
> happen for that; but that can be left for later...

Very well. I shall test this in practice :)

Thank you for the constructive reply :)

Yours Sincerely
Linux-Fan

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

--MP_/.NSW=EygP_k.2PxHI_y2FjO
Content-Type: text/x-patch
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
	filename=patch_cone_3_1_safe_signals_without_atomic.diff

diff -Nau -x cone -x Makefile.in -x Makefile -x .deps -x '*.cache' -x build=
version.H -x '*.log' -x config.status -r cone-0.92/cone/cone.C cone-0.92_ne=
w/cone/cone.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.000000000 +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 cone -x Makefile.in -x Makefile -x .deps -x '*.cache' -x build=
version.H -x '*.log' -x config.status -r cone-0.92/cone/curseshierarchy.C c=
one-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.000000000 +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 cone -x Makefile.in -x Makefile -x .deps -x '*.cache' -x build=
version.H -x '*.log' -x config.status -r cone-0.92/cone/curseshierarchy.H c=
one-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.000000000 +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 cone -x Makefile.in -x Makefile -x .deps -x '*.cache' -x build=
version.H -x '*.log' -x config.status -r cone-0.92/cone/Makefile.am cone-0.=
92_new/cone/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.000000000 +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 cone -x Makefile.in -x Makefile -x .deps -x '*.cache' -x build=
version.H -x '*.log' -x config.status -r cone-0.92/cone/masysmarefreshsigna=
l.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-02 16:58:02.000000000=
 +0100
@@ -0,0 +1,71 @@
+// BEGIN MASYSMA CHANGED
+
+#include "masysmarefreshsignal.H"
+
+// The idea behind maintaining a separate `signalsProcessed` and
+// `signalsReceived` counter is to avoid all possibilities of doing a conc=
urrent
+// write which might overwrite a pending signal: Think of a case where only
+// `signalProcessed` is used and the program receives a signal.
+// `signalProcessed` will increment to 1 leading to processHierarchyRefresh
+// scheduling a refresh and resetting the counter to 0. If in the meantime=
 there
+// was an update, it might not have been taken by `masysmaRefreshAllFolder=
s`
+// already but still, the counter might have just been reset AFTER the sig=
nal
+// was received meaning the update got lost. In order to prevent this, we
+// make sure the shared variable `signalsReceived` is never written
+// concurrently.
+
+sig_atomic_t MasysmaRefreshSignal::signalsProcessed =3D 0;
+CursesHierarchy* MasysmaRefreshSignal::masysmaHptr =3D NULL;
+
+// This counts the number of signals received and is /never/ written from
+// another location apart from masysmaReceiveRefreshSignal.
+static volatile sig_atomic_t signalsReceived =3D 0;
+
+// The signal handler is written in plain C instead of C++ as to keep
+// the function as simple as possible and leave as little potential for
+// interferences as possible.
+static void masysmaReceiveRefreshSignal(int arg0)
+{
+	signalsReceived++;
+}
+
+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 values of signalsReceived and signalsProcessed differ
+	// 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 the
+	// ifs are nested.
+	sig_atomic_t signalsReceivedAtCall =3D signalsReceived;
+	// This explicitly does not rely on any > or < comparison because one
+	// should be aware that by always counting up, the counters are expected
+	// to overflow (which is as long as comparison is only done with `!=3D`)
+	if(signalsReceivedAtCall !=3D MasysmaRefreshSignal::signalsProcessed) {
+		MasysmaRefreshSignal::signalsProcessed =3D signalsReceivedAtCall;
+		if(MasysmaRefreshSignal::masysmaHptr !=3D NULL)
+			MasysmaRefreshSignal::masysmaHptr->masysmaRefreshAllFolders();
+	}
+}
+
+// END MASYSMA CHANGED
diff -Nau -x cone -x Makefile.in -x Makefile -x .deps -x '*.cache' -x build=
version.H -x '*.log' -x config.status -r cone-0.92/cone/masysmarefreshsigna=
l.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-02 16:57:20.000000000=
 +0100
@@ -0,0 +1,19 @@
+#pragma once
+// BEGIN MASYSMA CHANGED
+#include "curseshierarchy.H"
+#include <signal.h>
+
+class MasysmaRefreshSignal {
+private:
+	// Pointer to the hierarchy (NULL if hierarchy screen not displayed)
+	static CursesHierarchy* masysmaHptr;
+	// Number of signals processed.
+	// This value is only ever modified from processHierarchyRefresh()
+	static sig_atomic_t signalsProcessed;
+public:
+	static void registerSignalHandler();
+	static void setHierarchyPointer(CursesHierarchy* hptr);
+	static void processHierarchyRefresh();
+};
+
+// END MASYSMA CHANGED
diff -Nau -x cone -x Makefile.in -x Makefile -x .deps -x '*.cache' -x build=
version.H -x '*.log' -x config.status -r cone-0.92/cone/myserver.C cone-0.9=
2_new/cone/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.000000000 +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_/.NSW=EygP_k.2PxHI_y2FjO--

--Sig_/JycL8HZ4S.LvDIAVHLb9rIY
Content-Type: application/pgp-signature
Content-Description: OpenPGP digital signature

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

iQIcBAEBCAAGBQJYk114AAoJEPhJcO2yngi1CykQAMkCGKGk1DLBVichyy2tzStN
irKj9L7CH85+23Fr+WSGGHdVNKnmSmVQueCSYa9ipBECFAZkp/FfRtDOWqv+o1D5
/bJnUKqnefIj1Jfki9ce0xeyg3kKabDH1TTJQal3ujJ7tdrHvPiEuqTCQwjVzy4K
xlNBAb6r+4x7VO5A8u5kYkpKTIRsGC1R8lShk9APlGM2lUzKiJamFHcsBPnU+KDP
znR1n5EszdnJ27vwrN5+rilXFIQRXTGBBA5nwta6xOhKdjbsHefgB50UcvaROZUT
PZkNKR+GIvuSdniT7o/J3vx/idpl1rh8RtL6INcmCj6VfUG3bXRdG62zR04agYFR
SKA4PiDxXjzlCfuoG4/DlYzdB2YN3g0cx/dqHHSKK6slg+rnVKW/eSa8YNC3NipR
94zYTQRWTAb00MsL79TwYxq6xxwY7m+iidZ6ltLTU+MDhIBqy8xILHUlpbHv3U93
05GQdfRo18WD52Ar1udzxt/WJKT7R3dhejCcHo7C5EfDACzSh840zjFlWFmmn6KY
nleGC/fv+NO+L3sduPQVNGS12iZHmWfGt5PUfvEXeUuU5wqCkqk+rlQXRzRi9r2S
gdE40X2dz24AoI9jaYT89VU6rsjV3Wc0cq8AykvQ3FirIbk6ZEV6SgU7HYGpkFiP
InfnwghxHyueFMZMw4ph
=kMS0
-----END PGP SIGNATURE-----

--Sig_/JycL8HZ4S.LvDIAVHLb9rIY--


--===============7466312358514830708==
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
--===============7466312358514830708==
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

--===============7466312358514830708==--