From dd68da2741fa63070d5ad206020dcccb9f429a5a Mon Sep 17 00:00:00 2001 From: Svetoslav Ganov Date: Fri, 27 Sep 2013 10:48:31 -0700 Subject: Print job files and print job records not always cleaned up. 1. We want the files for a print job to be removed as early as possible typically because the print job was cancelled, completed, the app or the spooler crashed during print job construction. We were keeping around in the spooler and hence to disc infos for jobs that are in final state since the app that created them may hold a reference to a local print job objec whose info it can access to get the latest print job state potentially after the job reached final state. The issue was that we were persisting to disc created print jobs which were during construction which requires careful handling for the various cases above. This is tricky and error prone. We used to tell the spooler to forget the print jobs infos when the app that created them died. The implementation to forget a print job was not careful and was nuking currently running print jobs in addition to the ones in a terminal state. Further, if the app dies before a print job is completed we were left with a stale print job in the spooler since we missed the signal to forget it (assuming we forget only inactive jobs). These issues suggest that the approach is problematic. Now we have a cache of print job infos for the jobs an app created. This cache is updated when the state of a print jobs changes using the new print job state observation code. When the app dies we remove the cached jobs for that app. Now if the app calls to get the print jobs it gets the cached ones, i.e. the print jobs it created during its lifetime, plus the print jobs that are still active fetched from the spooler. Note that transient state cannot be kept in the spooler since we unbind from it if there is no work and it may get killed. 2. Improved the print sub-system logging code to show the cached print job infos for apps and also dump the print job PDF file names. bug:10958357 Change-Id: I6f7c1968b6b7ba5be182a10df044ff7ea1fc3a61 --- .../android/printspooler/PrintSpoolerService.java | 127 ++++++++++++++------- 1 file changed, 83 insertions(+), 44 deletions(-) (limited to 'packages/PrintSpooler') diff --git a/packages/PrintSpooler/src/com/android/printspooler/PrintSpoolerService.java b/packages/PrintSpooler/src/com/android/printspooler/PrintSpoolerService.java index 62b35fe..87181f7 100644 --- a/packages/PrintSpooler/src/com/android/printspooler/PrintSpoolerService.java +++ b/packages/PrintSpooler/src/com/android/printspooler/PrintSpoolerService.java @@ -43,6 +43,7 @@ import android.print.PrintManager; import android.print.PrinterId; import android.print.PrinterInfo; import android.text.TextUtils; +import android.util.ArrayMap; import android.util.AtomicFile; import android.util.Log; import android.util.Slog; @@ -59,10 +60,12 @@ import org.xmlpull.v1.XmlPullParserException; import org.xmlpull.v1.XmlSerializer; import java.io.File; +import java.io.FileDescriptor; import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; +import java.io.PrintWriter; import java.util.ArrayList; import java.util.List; @@ -82,6 +85,8 @@ public final class PrintSpoolerService extends Service { private static final long CHECK_ALL_PRINTJOBS_HANDLED_DELAY = 5000; + private static final String PRINT_JOB_FILE_PREFIX = "print_job_"; + private static final String PRINT_FILE_EXTENSION = "pdf"; private static final Object sLock = new Object(); @@ -168,9 +173,9 @@ public final class PrintSpoolerService extends Service { PrintSpoolerService.this, 0, intent, PendingIntent.FLAG_ONE_SHOT | PendingIntent.FLAG_CANCEL_CURRENT).getIntentSender(); - Message message = mHandlerCaller.obtainMessageIIO( + Message message = mHandlerCaller.obtainMessageO( HandlerCallerCallback.MSG_ON_PRINT_JOB_STATE_CHANGED, - printJob.getAppId(), 0, printJob.getId()); + printJob); mHandlerCaller.executeOrSendMessage(message); message = mHandlerCaller.obtainMessageOO( @@ -179,9 +184,6 @@ public final class PrintSpoolerService extends Service { mHandlerCaller.executeOrSendMessage(message); printJob.setCreationTime(System.currentTimeMillis()); - synchronized (mLock) { - mPersistanceManager.writeStateLocked(); - } } @Override @@ -225,12 +227,40 @@ public final class PrintSpoolerService extends Service { } @Override - public void forgetPrintJobs(List printJobIds) { - PrintSpoolerService.this.forgetPrintJobs(printJobIds); + protected void dump(FileDescriptor fd, PrintWriter writer, String[] args) { + PrintSpoolerService.this.dump(fd, writer, args); } }; } + @Override + protected void dump(FileDescriptor fd, PrintWriter pw, String[] args) { + synchronized (mLock) { + String prefix = args[0]; + String tab = " "; + + pw.append(prefix).append("print jobs:").println(); + final int printJobCount = mPrintJobs.size(); + for (int i = 0; i < printJobCount; i++) { + PrintJobInfo printJob = mPrintJobs.get(i); + pw.append(prefix).append(tab).append(printJob.toString()); + pw.println(); + } + + pw.append(prefix).append("print job files:").println(); + File[] files = getFilesDir().listFiles(); + if (files != null) { + final int fileCount = files.length; + for (int i = 0; i < fileCount; i++) { + File file = files[i]; + if (file.isFile() && file.getName().startsWith(PRINT_JOB_FILE_PREFIX)) { + pw.append(prefix).append(tab).append(file.getName()).println(); + } + } + } + } + } + private void sendOnPrintJobQueued(PrintJobInfo printJob) { Message message = mHandlerCaller.obtainMessageO( HandlerCallerCallback.MSG_ON_PRINT_JOB_QUEUED, printJob); @@ -324,10 +354,9 @@ public final class PrintSpoolerService extends Service { case MSG_ON_PRINT_JOB_STATE_CHANGED: { if (mClient != null) { - PrintJobId printJobId = (PrintJobId) message.obj; - final int appId = message.arg1; + PrintJobInfo printJob = (PrintJobInfo) message.obj; try { - mClient.onPrintJobStateChanged(printJobId, appId); + mClient.onPrintJobStateChanged(printJob); } catch (RemoteException re) { Slog.e(LOG_TAG, "Error notify for print job state change.", re); } @@ -391,17 +420,46 @@ public final class PrintSpoolerService extends Service { public void createPrintJob(PrintJobInfo printJob) { synchronized (mLock) { addPrintJobLocked(printJob); + setPrintJobState(printJob.getId(), PrintJobInfo.STATE_CREATED, null); } } private void handleReadPrintJobsLocked() { + // Make a map with the files for a print job since we may have + // to delete some. One example of getting orphan files if the + // spooler crashes while constructing a print job. We do not + // persist partially populated print jobs under construction to + // avoid special handling for various attributes missing. + ArrayMap fileForJobMap = null; + File[] files = getFilesDir().listFiles(); + if (files != null) { + final int fileCount = files.length; + for (int i = 0; i < fileCount; i++) { + File file = files[i]; + if (file.isFile() && file.getName().startsWith(PRINT_JOB_FILE_PREFIX)) { + if (fileForJobMap == null) { + fileForJobMap = new ArrayMap(); + } + String printJobIdString = file.getName().substring(0, + PRINT_JOB_FILE_PREFIX.length()); + PrintJobId printJobId = PrintJobId.unflattenFromString( + printJobIdString); + fileForJobMap.put(printJobId, file); + } + } + } + final int printJobCount = mPrintJobs.size(); for (int i = 0; i < printJobCount; i++) { PrintJobInfo printJob = mPrintJobs.get(i); + // We want to have only the orphan files at the end. + if (fileForJobMap != null) { + fileForJobMap.remove(printJob.getId()); + } + // Update the notification. mNotificationController.onPrintJobStateChanged(printJob); - switch (printJob.getState()) { case PrintJobInfo.STATE_QUEUED: case PrintJobInfo.STATE_STARTED: @@ -415,6 +473,15 @@ public final class PrintSpoolerService extends Service { } break; } } + + // Delete the orphan files. + if (fileForJobMap != null) { + final int orphanFileCount = fileForJobMap.size(); + for (int i = 0; i < orphanFileCount; i++) { + File file = fileForJobMap.valueAt(i); + file.delete(); + } + } } public void checkAllPrintJobsHandled() { @@ -465,7 +532,7 @@ public final class PrintSpoolerService extends Service { } public File generateFileForPrintJob(PrintJobId printJobId) { - return new File(getFilesDir(), "print_job_" + return new File(getFilesDir(), PRINT_JOB_FILE_PREFIX + printJobId.flattenToString() + "." + PRINT_FILE_EXTENSION); } @@ -476,31 +543,6 @@ public final class PrintSpoolerService extends Service { } } - private void forgetPrintJobs(List printJobIds) { - synchronized (mLock) { - boolean printJobsRemoved = false; - final int removedPrintJobCount = printJobIds.size(); - for (int i = 0; i < removedPrintJobCount; i++) { - PrintJobId removedPrintJobId = printJobIds.get(i); - final int printJobCount = mPrintJobs.size(); - for (int j = printJobCount - 1; j >= 0; j--) { - PrintJobInfo printJob = mPrintJobs.get(j); - if (removedPrintJobId.equals(printJob.getId())) { - mPrintJobs.remove(j); - printJobsRemoved = true; - if (DEBUG_PRINT_JOB_LIFECYCLE) { - Slog.i(LOG_TAG, "[FORGOT] " + printJob.getId().flattenToString()); - } - removePrintJobFileLocked(printJob.getId()); - } - } - } - if (printJobsRemoved) { - mPersistanceManager.writeStateLocked(); - } - } - } - private void removeObsoletePrintJobs() { synchronized (mLock) { final int printJobCount = mPrintJobs.size(); @@ -523,7 +565,7 @@ public final class PrintSpoolerService extends Service { if (file.exists()) { file.delete(); if (DEBUG_PRINT_JOB_LIFECYCLE) { - Slog.i(LOG_TAG, "[REMOVE FILE FOR] " + printJobId.flattenToString()); + Slog.i(LOG_TAG, "[REMOVE FILE FOR] " + printJobId); } } } @@ -552,10 +594,7 @@ public final class PrintSpoolerService extends Service { switch (state) { case PrintJobInfo.STATE_COMPLETED: case PrintJobInfo.STATE_CANCELED: - // Just remove the file but keep the print job info since - // the app that created it may be holding onto the PrintJob - // instance and query it for its most recent state. We will - // remove the info for this job when told so by the system. + mPrintJobs.remove(printJob); removePrintJobFileLocked(printJob.getId()); // $fall-through$ @@ -582,9 +621,9 @@ public final class PrintSpoolerService extends Service { notifyOnAllPrintJobsHandled(); } - Message message = mHandlerCaller.obtainMessageIIO( + Message message = mHandlerCaller.obtainMessageO( HandlerCallerCallback.MSG_ON_PRINT_JOB_STATE_CHANGED, - printJob.getAppId(), 0, printJob.getId()); + printJob); mHandlerCaller.executeOrSendMessage(message); } } -- cgit v1.1