From 3e0774c0968dc38178cadb41558302c8a888b875 Mon Sep 17 00:00:00 2001
From: Jasmin Jessich <jasmin@anw.at>
Date: Thu, 25 May 2017 20:44:06 +0200
Subject: Fixed lock order in several files

- It is still not finished, but most of the locks should be in the right
  order now.
  I am still waiting for a VDR patch to check the lock order dynamically.
---
 pages/channels_widget.ecpp |  3 +--
 pages/epginfo.ecpp         |  3 +++
 pages/errors.vim           |  3 +++
 pages/ibox.ecpp            |  1 +
 pages/recordings.ecpp      |  8 ++++++--
 pages/schedule.ecpp        |  8 +++-----
 pages/searchresults.ecpp   | 12 ++++++------
 pages/timerconflicts.ecpp  | 13 ++++++-------
 pages/timers.ecpp          |  9 ++++-----
 9 files changed, 33 insertions(+), 27 deletions(-)
 create mode 100644 pages/errors.vim

(limited to 'pages')

diff --git a/pages/channels_widget.ecpp b/pages/channels_widget.ecpp
index 64dc3c9..f575f15 100644
--- a/pages/channels_widget.ecpp
+++ b/pages/channels_widget.ecpp
@@ -25,12 +25,11 @@ if (!logged_in && LiveSetup().UseAuth()) return reply.redirect("login.html");
 <%cpp>
 #if VDRVERSNUM >= 20301
 	LOCK_CHANNELS_READ;
-	if (false)
 #else
 	ReadLock channelsLock( Channels );
 	if ( !channelsLock )
-#endif
 		throw HtmlError( tr("Couldn't aquire access to channels, please try again later.") );
+#endif
 </%cpp>
 <select name="<$ name $>" id="<$ name $>" <{ reply.out() << ( !onchange.empty() ? "onchange=\""+onchange+"\"" : "" ); }>>
 %#if VDRVERSNUM >= 20301
diff --git a/pages/epginfo.ecpp b/pages/epginfo.ecpp
index 91e72f9..f177771 100644
--- a/pages/epginfo.ecpp
+++ b/pages/epginfo.ecpp
@@ -72,6 +72,9 @@ using namespace std;
 		// check for event:
 		else if (epgid.compare(0, event.length(), event) == 0) {
 #if VDRVERSNUM >= 20301
+			/* Need to lock here channels also, because CreateEpgInfo will lock
+			 * it also and this will result in a wrong lock order */
+			LOCK_CHANNELS_READ;
 			LOCK_SCHEDULES_READ;
 			epgEvent = EpgEvents::CreateEpgInfo(epgid, Schedules);
 #else
diff --git a/pages/errors.vim b/pages/errors.vim
new file mode 100644
index 0000000..31e5ff6
--- /dev/null
+++ b/pages/errors.vim
@@ -0,0 +1,3 @@
+make: menu.ecpp: Command not found
+make: channels_widget.ecpp: Command not found
+make: *** [channels_widget.cpp] Error 127
diff --git a/pages/ibox.ecpp b/pages/ibox.ecpp
index 947b746..2f58606 100644
--- a/pages/ibox.ecpp
+++ b/pages/ibox.ecpp
@@ -46,6 +46,7 @@ TimerConflictNotifier timerNotifier();
 	if (NowReplaying) {
 		RecordingsManagerPtr recManager = LiveRecordingsManager();
 #if VDRVERSNUM >= 20301
+		// is is OK to lock here, because CreateEpgInfo will *not* lock other lists
 		LOCK_RECORDINGS_READ;
 		cRecording *recording = (cRecording *)Recordings->GetByName(NowReplaying);
 #else
diff --git a/pages/recordings.ecpp b/pages/recordings.ecpp
index 615fb7c..f1ae8d6 100644
--- a/pages/recordings.ecpp
+++ b/pages/recordings.ecpp
@@ -147,8 +147,12 @@ if (!deleteResult.empty()) {
 			<div class="boxheader"><div><div><$ string(tr("List of recordings")) + " (" + diskinfo + ")" $></div></div></div>
 <%cpp>
 #if VDRVERSNUM >= 20301
-			LOCK_RECORDINGS_READ;
-			if (Recordings->Count() == 0) { // Access VDRs global cRecordings Recordings instance.
+			int rec_cnt;
+			{
+				LOCK_RECORDINGS_READ;
+				rec_cnt = Recordings->Count();  // Access VDRs global cRecordings Recordings instance.
+			}
+			if (rec_cnt == 0) {
 #else
 			if (Recordings.Count() == 0) { // Access VDRs global cRecordings Recordings instance.
 #endif
diff --git a/pages/schedule.ecpp b/pages/schedule.ecpp
index c5e5979..71b62a0 100644
--- a/pages/schedule.ecpp
+++ b/pages/schedule.ecpp
@@ -26,11 +26,8 @@ if (!logged_in && LiveSetup().UseAuth()) return reply.redirect("login.html");
 	pageTitle = trVDR("Schedule");
 
 #if VDRVERSNUM >= 20301
-        /* JJJ: Order wrong! Better get first the channel and with unlocked
-         *      channels lock read the schedules
-         */
-        LOCK_SCHEDULES_READ;
-        LOCK_CHANNELS_READ;
+	// we lock here only the channels on purpose
+	LOCK_CHANNELS_READ;
 #else
 	cSchedulesLock schedulesLock;
 	cSchedules const* schedules = cSchedules::Schedules( schedulesLock );
@@ -68,6 +65,7 @@ if (!logged_in && LiveSetup().UseAuth()) return reply.redirect("login.html");
 		throw HtmlError( tr("Couldn't find channel or no channels available. Maybe you mistyped your request?") );
 
 #if VDRVERSNUM >= 20301
+	LOCK_SCHEDULES_READ;
 	cSchedule const* Schedule = Schedules->GetSchedule( (const cChannel *)Channel );
 #else
 	cSchedule const* Schedule = schedules->GetSchedule( Channel );
diff --git a/pages/searchresults.ecpp b/pages/searchresults.ecpp
index 8970f37..6e554ac 100644
--- a/pages/searchresults.ecpp
+++ b/pages/searchresults.ecpp
@@ -51,9 +51,9 @@ bool logged_in(false);
 		<& pageelems.logo &>
 		<& menu active=("searchepg") &>
 		<div class="inhalt">
-%           if (results.size() == 0) {
-                <$ tr("No search results") $>
-%           }
+%			if (results.size() == 0) {
+				<$ tr("No search results") $>
+%			}
 			<table class="listing" cellspacing="0" cellpadding="0">
 <%cpp>
 				string current_day = "";
@@ -63,9 +63,9 @@ bool logged_in(false);
 #endif
 				for (SearchResults::iterator result = results.begin(); result != results.end(); ++result) {
 #if VDRVERSNUM >= 20301
-				        cChannel* channel = (cChannel *)Channels->GetByChannelID(result->Channel());
+					cChannel* channel = (cChannel *)Channels->GetByChannelID(result->Channel());
 #else
-				        cChannel* channel = Channels.GetByChannelID(result->Channel());
+					cChannel* channel = Channels.GetByChannelID(result->Channel());
 #endif
 					if (!channel) continue;
 					string channelname = channel->Name();
@@ -99,7 +99,7 @@ bool logged_in(false);
 					</tr>
 %						current_day = day;
 %					}
-                    <tr>
+					<tr>
 						<td class="action leftcol <? bottom ? "bottomrow"?>"><& pageelems.event_timer epgid=(epgid) &></td>
 						<td class="topaligned <? bottom ? "bottomrow"?>"><div class="withmargin"><a href="schedule.html?channel=<$ channelnr $>"><$ channelname $></a></div></td>
 						<td class="topaligned <? bottom ? "bottomrow"?>"><div class="withmargin nowrap"><$ start $> - <$ end $></div></td>
diff --git a/pages/timerconflicts.ecpp b/pages/timerconflicts.ecpp
index 61e7338..bbc35b6 100644
--- a/pages/timerconflicts.ecpp
+++ b/pages/timerconflicts.ecpp
@@ -44,11 +44,6 @@ if (!logged_in && LiveSetup().UseAuth()) return reply.redirect("login.html");
 %			} else {
 			<table class="listing" cellspacing="0" cellpadding="0">
 <%cpp>
-#if VDRVERSNUM >= 20301
-				/* JJJ: Lock this later when it is realy used */
-				LOCK_TIMERS_READ;
-				LOCK_SCHEDULES_READ;
-#endif
 				for (TimerConflicts::iterator conflict = timerConflicts.begin(); conflict != timerConflicts.end(); ++conflict) {
 					const std::list< TimerInConflict >& conflTimers = conflict->ConflictingTimers();
 					for (std::list< TimerInConflict >::const_iterator confltimer = conflTimers.begin(); confltimer != conflTimers.end(); ++confltimer) {
@@ -73,6 +68,7 @@ if (!logged_in && LiveSetup().UseAuth()) return reply.redirect("login.html");
 <%cpp>
 						for (std::list<int>::const_iterator timerIndex = confltimer->concurrentTimerIndices.begin(); timerIndex != confltimer->concurrentTimerIndices.end(); ++timerIndex) {
 #if VDRVERSNUM >= 20301
+							LOCK_TIMERS_READ;
 							cTimer* timer = (cTimer *)Timers->Get(*timerIndex-1);
 #else
 							cTimer* timer = Timers.Get(*timerIndex-1);
@@ -94,11 +90,14 @@ if (!logged_in && LiveSetup().UseAuth()) return reply.redirect("login.html");
 								timerStateHint = tr("Timer is active.");
 							}
 
-						        EpgInfoPtr epgEvent;
+							EpgInfoPtr epgEvent;
 							string longDescription;
 							string title;
 #if VDRVERSNUM >= 20301
-							if (!timer->Event()) timer->SetEventFromSchedule(Schedules);
+							if (!timer->Event()) {
+								LOCK_SCHEDULES_READ;
+								timer->SetEventFromSchedule(Schedules);
+							}
 #else
 							if (!timer->Event()) timer->SetEventFromSchedule();
 #endif
diff --git a/pages/timers.ecpp b/pages/timers.ecpp
index 771a907..ff3fd40 100644
--- a/pages/timers.ecpp
+++ b/pages/timers.ecpp
@@ -75,15 +75,14 @@ static const size_t maximumDescriptionLength = 300;
 			<table class="listing" cellspacing="0" cellpadding="0">
 <%cpp>
 				// output of the timer list:
-#if VDRVERSNUM >= 20301
-				/* JJJ: Lock this inside the loop */
-				LOCK_SCHEDULES_READ;
-#endif
 				for (SortedTimers::iterator timer = timers.begin(); timer != timers.end(); ++timer) {
 				    EpgInfoPtr epgEvent;
 					string longDescription;
 #if VDRVERSNUM >= 20301
-					if (!timer->Event()) timer->SetEventFromSchedule(Schedules);
+					if (!timer->Event()) {
+						LOCK_SCHEDULES_READ;
+						timer->SetEventFromSchedule(Schedules);
+					}
 #else
 					if (!timer->Event()) timer->SetEventFromSchedule();
 #endif
-- 
cgit v1.2.3