Fix imap_pool hanging when getting a second MailBox (+stuff)

In the following situation:

- the pool has handed out one or more mailboxes already,
- if you'd call get_box(),
- imap_pool would hang.

the reason for this is that when you call get_box(), the pool checks if
all connections are still alive to prevent handing out dead ones. it'd
also check all boxes that were already handed out and in use, resulting
in hanging the thread and eventually deadlocking.

This commit:

- removes the list of handed out boxes from the alive check loop,
- removes commented out log messages that were present for finding this
  bug,
- adds a list where it'll track which threads are currently holding a
  mailbox.

1 files changed, 10 insertions, 9 deletions

diff --git a/src/acit/imap_pool.py b/src/acit/imap_pool.py
index af541dd..c592225 100644
--- a/src/acit/imap_pool.py
+++ b/src/acit/imap_pool.py
@@ -5,7 +5,7 @@ from imaplib import IMAP4
 
 from imap_tools import MailBox
 
-from threading import Event, RLock
+from threading import Event, RLock, current_thread
 
 
 class PoolEmpty(Exception):
@@ -53,6 +53,7 @@ class MailBoxPool():
 		self.errors=[]
 
 		self.initalised=Event()
+		self.holding_threads=[]
 
 		self.lock=RLock()
 		self.boxreturned=Event()
@@ -88,13 +89,14 @@ class MailBoxPool():
 		#self.log("Waiting for lock")
 		with self.lock:
 			toremove=[]
-			for mb in self.pool+self.taken:
+			for mb in self.pool:
+				self.log(mb)
 				if not self.is_alive(mb):
 					toremove.append(mb)
 
 			for mb in toremove:
 				try:
-					mb.logout() # logout if needed
+					MailBox.logout(mb) # logout if needed
 				except:
 					pass
 				if mb in self.pool: self.pool.remove(mb)
@@ -126,9 +128,7 @@ class MailBoxPool():
 		"Gets a new mailbox from the pool"
 		self.initalised.wait()
 		while True:
-			#self.log("Waiting for lock (available=%d,taken=%d)"%(len(self.pool),len(self.taken))) # NOTE:testlog
 			with self.lock:
-				#self.log("Aquired") # NOTE:testlog
 				if self.get_pool_size()<1:
 					raise PoolEmpty("No connections in pool!")
 				self.ensure_all_connections()
@@ -136,31 +136,32 @@ class MailBoxPool():
 					self.boxreturned.clear()
 					mb=self.pool.pop(0)
 					self.taken.append(mb)
+					self.holding_threads.append(current_thread())
 					return mb
-				#self.log("No boxes") # NOTE:testlog
 
 			self.boxreturned.wait()
 	
 	def release(self,mb:PooledMailBox):
 		"Returns a mailbox back to the pool. Please use a context manager instead of manually releasing."
 		self.initalised.wait()
-		#self.log("Waiting for lock, trying to release a connection") # NOTE:testlog
 		with self.lock:
-			#self.log("Aquired") # NOTE:testlog
 			if mb in self.taken:
 				self.taken.remove(mb)
+			self.holding_threads.remove(current_thread())
 			self.pool.append(mb)
 			self.boxreturned.set()
 	
 	def close(self):
 		"Closes all the mailboxes"
 		self.initalised.set() # to force an exit wherever possible
+		self.n=0 # make sure no new mailboxes get created
 		while len(self.pool)+len(self.taken)>0:
 			with self.lock:
 				while len(self.pool)>0:
 					mb=self.pool.pop(0)
 					MailBox.logout(mb) # pass it this way bc we overwrite logout()
-			self.boxreturned.wait(.5)
+			self.log("Threads still holding a MailBox:",", ".join([thread.name for thread in self.holding_threads]))
+			self.boxreturned.wait()