Skip to content

Commit

Permalink
fix b0rk introduced by commit SHA-1: bcd73cd :: we don't do a *revert…
Browse files Browse the repository at this point in the history 
…* action per se, but rather improve upon the patch we picked up there from the experimental branch: as it turns out, the patch caused a lot of trouble which has been resolved to allow the running background task(s) access to a reduced clone of the WebLibraryDetails, which does not exhibit the cyclic dependency that the original WebLibraryDetails instance incorporated, thus breaking the cyclic reference and allowing the .NET GC to do its job on the Library instance(s) ASAP.

As this problem was discovered while doing work on commit SHA-1: ed2cb58, these files also include the remainder of the work done for that commit, as it was important to separate out the patches which fixed the cyclic memory reference.
  • Loading branch information
@GerHobbelt
GerHobbelt committed Nov 1, 2019
1 parent ed2cb58 commit 95dff9b
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 150 deletions.
223 changes: 96 additions & 127 deletions Qiqqa/DocumentLibrary/Library.cs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using Qiqqa.Common.Configuration;
using Qiqqa.Common.TagManagement;
using Qiqqa.DocumentConversionStuff;
Expand All @@ -18,113 +16,53 @@
using Utilities.Files;
using Utilities.GUI;
using Utilities.Misc;
using Utilities.Strings;
using File = Alphaleonis.Win32.Filesystem.File;
using Directory = Alphaleonis.Win32.Filesystem.Directory;
using File = Alphaleonis.Win32.Filesystem.File;
using Path = Alphaleonis.Win32.Filesystem.Path;


namespace Qiqqa.DocumentLibrary
{
public class Library : IDisposable
{
public override string ToString()
{
return String.Format("Library: {0}", this.web_library_detail.Title);
return String.Format("Library: {0}", web_library_detail.Title);
}

/// <summary>
/// Use this singleton instance ONLY for testing purposes!
/// </summary>
public static Library GuestInstance
{
get
{
return WebLibraryManager.Instance.Library_Guest;
}
}
public static Library GuestInstance => WebLibraryManager.Instance.Library_Guest;

private WebLibraryDetail web_library_detail;
public WebLibraryDetail WebLibraryDetail
{
get
{
return web_library_detail;
}
}
public WebLibraryDetail WebLibraryDetail => web_library_detail;

private LibraryDB library_db;
public LibraryDB LibraryDB
{
get
{
return library_db;
}
}
public LibraryDB LibraryDB => library_db;

private FolderWatcherManager folder_watcher_manager;
public FolderWatcherManager FolderWatcherManager
{
get
{
return folder_watcher_manager;
}
}
public FolderWatcherManager FolderWatcherManager => folder_watcher_manager;

private LibraryIndex library_index;
public LibraryIndex LibraryIndex
{
get
{
return library_index;
}
}
public LibraryIndex LibraryIndex => library_index;

private AITagManager ai_tag_manager;
public AITagManager AITagManager
{
get
{
return ai_tag_manager;
}
}
public AITagManager AITagManager => ai_tag_manager;

private RecentlyReadManager recently_read_manager;
public RecentlyReadManager RecentlyReadManager
{
get
{
return recently_read_manager;
}
}
public RecentlyReadManager RecentlyReadManager => recently_read_manager;

private BlackWhiteListManager blackwhite_list_manager;
public BlackWhiteListManager BlackWhiteListManager
{
get
{
return blackwhite_list_manager;
}
}
public BlackWhiteListManager BlackWhiteListManager => blackwhite_list_manager;

PasswordManager password_manager;
public PasswordManager PasswordManager
{
get
{
return password_manager;
}
}
private PasswordManager password_manager;
public PasswordManager PasswordManager => password_manager;

ExpeditionManager expedition_manager;
public ExpeditionManager ExpeditionManager
{
get
{
return expedition_manager;
}
}
private ExpeditionManager expedition_manager;
public ExpeditionManager ExpeditionManager => expedition_manager;

Dictionary<string, PDFDocument> pdf_documents = new Dictionary<string, PDFDocument>();
private Dictionary<string, PDFDocument> pdf_documents = new Dictionary<string, PDFDocument>();
private object pdf_documents_lock = new object();

public delegate void OnLibraryLoadedHandler(Library library);
Expand Down Expand Up @@ -214,14 +152,14 @@ public Library(WebLibraryDetail web_library_detail)
Directory.CreateDirectory(LIBRARY_BASE_PATH);
Directory.CreateDirectory(LIBRARY_DOCUMENTS_BASE_PATH);

this.library_db = new LibraryDB(this.LIBRARY_BASE_PATH);
this.folder_watcher_manager = new FolderWatcherManager(this);
this.library_index = new LibraryIndex(this);
this.ai_tag_manager = new AITagManager(this);
this.recently_read_manager = new RecentlyReadManager(this);
this.blackwhite_list_manager = new BlackWhiteListManager(this);
this.password_manager = new PasswordManager(this);
this.expedition_manager = new ExpeditionManager(this);
library_db = new LibraryDB(LIBRARY_BASE_PATH);
folder_watcher_manager = new FolderWatcherManager(this);
library_index = new LibraryIndex(this);
ai_tag_manager = new AITagManager(this);
recently_read_manager = new RecentlyReadManager(this);
blackwhite_list_manager = new BlackWhiteListManager(this);
password_manager = new PasswordManager(this);
expedition_manager = new ExpeditionManager(this);

// Start loading the documents in the background...
SafeThreadPool.QueueUserWorkItem(o => BuildFromDocumentRepository());
Expand All @@ -231,30 +169,51 @@ public Library(WebLibraryDetail web_library_detail)
//
// Once completed, an event will be fired to
// help the main application update any relevant views.
void BuildFromDocumentRepository()
private void BuildFromDocumentRepository()
{
try
{
LibraryIsLoaded = false;

// abort work when this library instance has already been Dispose()d in the main UI thread:
if (LibraryIsKilled)
{
Logging.Info("Building the library has been SKIPPED/ABORTED as the library {0} has already been killed.", WebLibraryDetail.Id);
return;
}

Stopwatch clk = Stopwatch.StartNew();
long prev_clk = 0;
long elapsed = 0;
Logging.Debug特("+Build library from repository");
List<LibraryDB.LibraryItem> library_items = this.library_db.GetLibraryItems(null, PDFDocumentFileLocations.METADATA);
List<LibraryDB.LibraryItem> library_items = library_db.GetLibraryItems(null, PDFDocumentFileLocations.METADATA);

// abort work when this library instance has already been Dispose()d in the main UI thread:
if (LibraryIsKilled)
{
Logging.Info("Building the library has been SKIPPED/ABORTED as the library {0} has already been killed.", WebLibraryDetail.Id);
return;
}

elapsed = clk.ElapsedMilliseconds;
Logging.Debug特(":Build library '{2}' from repository -- time spent: {0} ms on fetching {1} records from SQLite DB.", elapsed, library_items.Count, this.WebLibraryDetail.DescriptiveTitle);
Logging.Debug特(":Build library '{2}' from repository -- time spent: {0} ms on fetching {1} records from SQLite DB.", elapsed, library_items.Count, WebLibraryDetail.DescriptiveTitle);
prev_clk = elapsed;

// Get the annotations cache
Dictionary<string, byte[]> library_items_annotations_cache = this.library_db.GetLibraryItemsAsCache(PDFDocumentFileLocations.ANNOTATIONS);
Dictionary<string, byte[]> library_items_annotations_cache = library_db.GetLibraryItemsAsCache(PDFDocumentFileLocations.ANNOTATIONS);

// abort work when this library instance has already been Dispose()d in the main UI thread:
if (LibraryIsKilled)
{
Logging.Info("Building the library has been SKIPPED/ABORTED as the library {0} has already been killed.", WebLibraryDetail.Id);
return;
}

elapsed = clk.ElapsedMilliseconds;
Logging.Debug特(":Build library '{2}' from repository -- time spent: {0} ms on fetching annotation cache for {1} records.", elapsed - prev_clk, library_items.Count, this.WebLibraryDetail.DescriptiveTitle);
Logging.Debug特(":Build library '{2}' from repository -- time spent: {0} ms on fetching annotation cache for {1} records.", elapsed - prev_clk, library_items.Count, WebLibraryDetail.DescriptiveTitle);
prev_clk = elapsed;

Logging.Info("Library '{2}': Loading {0} files from repository at {1}", library_items.Count, LIBRARY_DOCUMENTS_BASE_PATH, this.WebLibraryDetail.DescriptiveTitle);
Logging.Info("Library '{2}': Loading {0} files from repository at {1}", library_items.Count, LIBRARY_DOCUMENTS_BASE_PATH, WebLibraryDetail.DescriptiveTitle);

for (int i = 0; i < library_items.Count; ++i)
{
Expand All @@ -264,15 +223,17 @@ void BuildFromDocumentRepository()
elapsed = clk.ElapsedMilliseconds;
if (prev_clk + 1000 <= elapsed)
{
StatusManager.Instance.UpdateStatus("LibraryInitialLoad", String.Format("Loading your library '{0}'", this.WebLibraryDetail.DescriptiveTitle), i, library_items.Count);
Logging.Info("Library '{2}': Loaded {0}/{1} documents", i, library_items.Count, this.WebLibraryDetail.DescriptiveTitle);
StatusManager.Instance.UpdateStatus("LibraryInitialLoad", String.Format("Loading your library '{0}'", WebLibraryDetail.DescriptiveTitle), i, library_items.Count);
Logging.Info("Library '{2}': Loaded {0}/{1} documents", i, library_items.Count, WebLibraryDetail.DescriptiveTitle);
prev_clk = elapsed;

System.Threading.Thread.Yield();
}

if (LibraryIsKilled)
{
// abort work when this library instance has already been Dispose()d in the main UI thread:
Logging.Info("Building the library has been SKIPPED/ABORTED as the library {0} has already been killed.", WebLibraryDetail.Id);
break;
}

Expand All @@ -282,23 +243,23 @@ void BuildFromDocumentRepository()
}
catch (Exception ex)
{
Logging.Error(ex, "Library '{1}': There was a problem loading document {0}", library_item, this.WebLibraryDetail.DescriptiveTitle);
Logging.Error(ex, "Library '{1}': There was a problem loading document {0}", library_item, WebLibraryDetail.DescriptiveTitle);
}
}

StatusManager.Instance.ClearStatus("LibraryInitialLoad");

Logging.Debug特("-Build library '{2}' from repository -- time spent: {0} ms on {1} library records.", clk.ElapsedMilliseconds, library_items.Count, this.WebLibraryDetail.DescriptiveTitle);
Logging.Debug特("-Build library '{2}' from repository -- time spent: {0} ms on {1} library records.", clk.ElapsedMilliseconds, library_items.Count, WebLibraryDetail.DescriptiveTitle);
}
catch (Exception ex)
{
if (LibraryIsKilled)
{
Logging.Warn(ex, "There was a failure while building the *KILLED* document library instance for library {0} ({1})", this.WebLibraryDetail.DescriptiveTitle, this.WebLibraryDetail.Id);
Logging.Warn(ex, "There was a failure while building the *KILLED* document library instance for library {0} ({1})", WebLibraryDetail.DescriptiveTitle, WebLibraryDetail.Id);
}
else
{
Logging.Error(ex, "There was a problem while building the document library {0} ({1})", this.WebLibraryDetail.DescriptiveTitle, this.WebLibraryDetail.Id);
Logging.Error(ex, "There was a problem while building the document library {0} ({1})", WebLibraryDetail.DescriptiveTitle, WebLibraryDetail.Id);
}
}
finally
Expand Down Expand Up @@ -932,8 +893,8 @@ internal void NotifyLibraryThatDocumentHasChangedExternally(string fingerprint)
Logging.Info("Library has been notified that {0} has changed", fingerprint);
try
{
LibraryDB.LibraryItem library_item = this.library_db.GetLibraryItem(fingerprint, PDFDocumentFileLocations.METADATA);
this.LoadDocumentFromMetadata(library_item, null, true);
LibraryDB.LibraryItem library_item = library_db.GetLibraryItem(fingerprint, PDFDocumentFileLocations.METADATA);
LoadDocumentFromMetadata(library_item, null, true);
}
catch (Exception ex)
{
Expand All @@ -954,7 +915,7 @@ public class PDFDocumentEventArgs : EventArgs
{
public PDFDocumentEventArgs(PDFDocument pdf_document)
{
this.PDFDocument = pdf_document;
PDFDocument = pdf_document;
}

public PDFDocument PDFDocument { get; }
Expand Down Expand Up @@ -1043,13 +1004,7 @@ public static string GetLibraryBasePathForId(string id)
return Path.GetFullPath(Path.Combine(ConfigurationManager.Instance.BaseDirectoryForQiqqa, id));
}

public string LIBRARY_BASE_PATH
{
get
{
return GetLibraryBasePathForId(this.web_library_detail.Id);
}
}
public string LIBRARY_BASE_PATH => GetLibraryBasePathForId(web_library_detail.Id);

public string LIBRARY_DOCUMENTS_BASE_PATH
{
Expand All @@ -1067,13 +1022,7 @@ public string LIBRARY_DOCUMENTS_BASE_PATH
}
}

public string LIBRARY_INDEX_BASE_PATH
{
get
{
return Path.GetFullPath(Path.Combine(LIBRARY_BASE_PATH, @"index"));
}
}
public string LIBRARY_INDEX_BASE_PATH => Path.GetFullPath(Path.Combine(LIBRARY_BASE_PATH, @"index"));

#endregion

Expand Down Expand Up @@ -1106,8 +1055,8 @@ protected virtual void Dispose(bool disposing)
// Do we need to check that the library has finished being loaded?

// Switch off the living things
this.library_index?.Dispose();
this.folder_watcher_manager?.Dispose();
library_index?.Dispose();
folder_watcher_manager?.Dispose();

// NULL the memory database
Utilities.LockPerfTimer l2_clk = Utilities.LockPerfChecker.Start();
Expand All @@ -1120,21 +1069,41 @@ protected virtual void Dispose(bool disposing)
}

// Clear the references for sanity's sake
this.expedition_manager = null;
this.password_manager = null;
this.blackwhite_list_manager = null;
this.recently_read_manager = null;
this.ai_tag_manager = null;
this.library_index = null;
this.folder_watcher_manager = null;
this.library_db = null;
expedition_manager = null;
password_manager = null;
blackwhite_list_manager = null;
recently_read_manager = null;
ai_tag_manager = null;
library_index = null;
folder_watcher_manager = null;
library_db = null;

this.web_library_detail = null; // cyclic reference as WebLibraryDetail instance reference us, so we MUST nil this one to break the cycle for the GC to work well.
#if false
web_library_detail = null; // cyclic reference as WebLibraryDetail instance reference us, so we MUST nil this one to break the cycle for the GC to work well.
#else
// cyclic reference as WebLibraryDetail instance reference us, so we MUST nil this one to break the cycle for the GC to work well.
//
// WARNING:
// The most obvious way (see above in `#if false` branch) would be to NULL the weblibdetail reference, but this will cause all sorts of extremely
// nasty crashes, including memory corruption, as this reference is accessed in Library background task(s) which might discover that the
// library at hand has been killed rather late.
//
// When those code chunks, e.g. *anything* inside `BuildFromDocumentRepository()`, crash on a NULL dereference of any of the other NULLed
// library members, the error resolution code highly depends on a *still working* web_library_detail reference/instance.
// To resolve the cyclic reference in there (as the web_lib_detail has a `Library` reference), we hack this by creating a *temporary*
// intermediate web_library_detail instance, which is a copy of the original *sans Library reference*.
// We DO NOT nuke the Library member in the original web_library_detail as that would cause all sorts of other harm since there's other
// code which depends on a certain valid lifetime of that instance and that code should dispose of the record once it is done using it...
//
// Cloning...
web_library_detail = web_library_detail.CloneSansLibraryReference();

#endif

++dispose_count;
}

#endregion
#endregion

}
}
Loading

0 comments on commit 95dff9b

Please sign in to comment.