[Xenophon]

Originally Posted by itimpi

Nook mode tends to get very limited testing. I believe that David Pierron who wrote the Nook specific code no longer has the Nook he had at the time. When you compound this with the fact that calibre2opds developers such as myself do not (currently at least) even have a mac for testing against the combination of Nook mode plus OSX is always likely to throw up something new.

In terms of the heap space it might be worth looking at the RunGui.sh file to see if increasing the parameters for heap space allocation helps. This is the first release for which explicit values have been specified as we found that the defaults tended to be too low. The values used were chosen from ones that seemed to work well on Linux and Windows and may not be ideal for the Mac. You will see that an inital heap size of 128MB is specified with a maximum of 512MB. It is not clear to me if the initial value should be higher, or the maximum value. I did not want to set the maximum too as it can cause problems for those with less RAM than more modern systems tend to have. The values chosen have easily handled libraries with over 20K books in testing (albeit that testing was not done in nook mode).

messing with rungui.sh is not fruitful on the Mac. Instead, you need to modify the values in the Info.plist file instead -- that's where the VM arguments are set. Specifically, change

Spoiler:

		<dict>
			<key>apple.laf.useScreenMenuBar</key>
			<string>true</string>
		</dict>

to

Spoiler:

		<dict>
			<key>apple.laf.useScreenMenuBar</key>
			<string>true</string>
		</dict>
		<key>VMOptions</key>
		<string>-Xms128m -Xmx512m</string>

Originally Posted by itimpi

It might be instructive to see if you have any such issue if running in the default mode as that tends by its very nature to be the one that gets the most exhaustive testing.

No problems when running in any of the non-Nook modes.

Originally Posted by itimpi

I will be a bit worried if rev 143 built from svn produces different results to rev143 built on Windows and then packaged into the dmg. Being java they should be identical. Could you please point out where you got this NPE - it did not seem to happen when I ran in Nook mode on my Windows system. My guess is that there is some variable that needs initialising to a slightly different setting if running in Nook mode, but the problem is going to spot which one if I cannot reproduce the NPE.


This is easy enough to protect against by adding an explicit check for null. However more to the point is why that value can be null in the first place. It suggests to me that there is something else going on. Do you know if this crash ever occurs if not in Nook mode? I am wondering if there is an issue with setting the path to save the CRC cache to in that mode.

All I know about the Nook mode is from examining the code. That makes it much harder to spot logic flaws as it is not always clear what should be happening in that mode that is different to other modes. If you spot any strange behavior please feel free to point it out. As I discover more about the Nook specific behavior I am tending to add comments to the code so that those examining it later have a better view of what is meant to be happening.

Indeed, the problem turns out to be the path for saving the CRC cache. Here're diffs for the two files that need some fixing.
CachedFileManager.java:

Spoiler:

Index: DataModel/src/main/java/com/gmail/dpierron/calibre/cache/CachedFileManager.java
===================================================================
--- DataModel/src/main/java/com/gmail/dpierron/calibre/cache/CachedFileManager.java	(revision 143)
+++ DataModel/src/main/java/com/gmail/dpierron/calibre/cache/CachedFileManager.java	(working copy)
@@ -136,6 +136,12 @@
    */
   public void setCacheFolder(File cf) {
     cacheFolder = cf;
+    logger.error("Setting cacheFolder and CacheFile");
+    if (cacheFolder == null) {
+	logger.error("Incoming cf was null");
+    } else {
+	logger.error("Incoming cf specified path: " + cacheFolder.getAbsolutePath());
+    }
     cacheFile = new File(cacheFolder.getAbsolutePath(), "calibre2opds.cache");
     logger.debug("CRC Cache file set to " + cacheFile.getPath());
   }
@@ -161,9 +167,25 @@
     FileOutputStream fs = null;
     try {
       try {
+	  if (cacheFile == null) {
+	      logger.error("cacheFile is null!!!");
+	  } else {
+	      logger.debug("have a cacheFile.");
+	      logger.debug("cacheFile is: " + cacheFile);
+	  }
         logger.debug("STARTED Saving CRC cache to file " + cacheFile.getPath());
         fs = new FileOutputStream(cacheFile);
+	if (fs == null) {
+	    logger.error("null FileOutputStream for CRC cache file " + cacheFile.getPath()); 
+	} else {
+	    logger.debug("created fs: " + fs);
+	}
         os = new ObjectOutputStream(fs);
+	if (os == null) {
+	    logger.error("null ObjectOutputStream for CRC cache file " + cacheFile.getPath());
+	} else {
+	    logger.debug("created os: " + os);
+	}
 
         // Write out the cache entries
         for (Map.Entry<String, CachedFile> m : cachedFilesMap.entrySet()) {
@@ -200,8 +222,16 @@
         }
       } finally {
         try {
-          os.close();
-          fs.close();
+	  if (os == null) {
+	    logger.fatal("ObjectOutputStream os unexpectedly null. Is the cacheFolder set to the correct path?"); 
+	  } else {
+	    os.close();
+	  }
+	  if (fs == null) {
+	    logger.fatal("FileOutputStream fs unexpectedly null. Is the cacheFolder set to the correct path?"); 
+	  } else {
+	    fs.close();
+	  }
         } catch (IOException e) {
           // Do nothing - we ignore an error at this point
           // Having said that, an error here is a bit unexpected

and Catalog.java:

Spoiler:

Index: OpdsOutput/src/main/java/com/gmail/dpierron/calibre/opds/Catalog.java
===================================================================
--- OpdsOutput/src/main/java/com/gmail/dpierron/calibre/opds/Catalog.java	(revision 143)
+++ OpdsOutput/src/main/java/com/gmail/dpierron/calibre/opds/Catalog.java	(working copy)
@@ -500,10 +500,15 @@
         catalogParentFolder = calibreLibraryFolder;
       }
       logger.trace("catalogParentFolder set to " + catalogParentFolder);
-      File catalogFolder = new File(catalogParentFolder, currentProfile.getCatalogFolderName());
+      StringBuilder sb = new StringBuilder();
+      sb.append(currentProfile.getCatalogFolderName());
+      if (currentProfile.getDeviceMode() == DeviceMode.Nook) {
+	  // Remember to add the trook folder extension when we are in trook mode!
+	  sb.append(Constants.TROOK_FOLDER_EXTENSION);
+      }
+      File catalogFolder = new File(catalogParentFolder, sb.toString());
       if (logger.isTraceEnabled())
         logger.trace("New catalog to be generated at " + catalogFolder.getPath());
-
       // If copying catalog back to database folder check it is safe to overwrite
       if (true == currentProfile.getCopyToDatabaseFolder()) {
         File targetFolder = currentProfile.getDatabaseFolder();
@@ -1069,7 +1074,11 @@
           }
           // when publishing to the Nook, don't forget to copy the search database
           File destinationFile = new File(targetFolder, Constants.TROOK_SEARCH_DATABASE_FILENAME);
-          Helper.copy(TrookSpecificSearchDatabaseManager.INSTANCE.getDatabaseFile(), destinationFile);
+	  final File srcSearchFile = 
+	      TrookSpecificSearchDatabaseManager.INSTANCE.getDatabaseFile();
+          if (srcSearchFile != null) {
+	      Helper.copy(srcSearchFile, destinationFile);
+	  }
           // Also need to make sure catalog.xml exists for Trook use
           // Use index.xml already generated
           // logger.trace("dewstinationFolder=" + destinationFolder);
@@ -1208,11 +1217,12 @@
       logger.error(" ");
       logger.error("*************************************************");
       logger.error(Localization.Main.getText("error.unexpectedFatal").toUpperCase());
-      logger.error(Localization.Main.getText("error.cause").toUpperCase() + ": " + t.getCause());
+      logger.error(Localization.Main.getText("error.cause").toUpperCase() + ": " + t + ": " + t.getCause());
       logger.error(Localization.Main.getText("error.message").toUpperCase() + ": " + t.getMessage());
-      //logger.error(Localization.Main.getText("error.stackTrace").toUpperCase() + ":\n" + Helper.getStackTrace(t));
+      logger.error(Localization.Main.getText("error.stackTrace").toUpperCase() + ":\n" + Helper.getStackTrace(t));
       // ITIMPI: Need to check following line prints good stack trace - if not re-instate above line
-      logger.error(Localization.Main.getText("error.stackTrace").toUpperCase() + ":\n" + t);
+      // DFS: Nope!  It doesn't work. (At least not on Mac OS X 10.7.3, with Java 1.6).
+      //logger.error(Localization.Main.getText("error.stackTrace").toUpperCase() + ":\n" + t);
       logger.error("*************************************************");
       logger.error(" ");
     } finally {

Originally Posted by itimpi

I really would like to get these issues ironed out so that the 3.0 release an go into more widespread circulation. The earlier releases are now so far from the current code base that even investigating issues raised against them is getting to be more difficult.

And I'm super-eager to have a working 3.0 release that I can use with my nook. Hence, the above fixes.

Please note: I have yet to check whether the resulting output actually works on the nook. I've only gotten as far as making the crash go away.

Xenophon