delete incomplete filelists

[paka]

I have the feature "Don't delete file lists when exiting" switched on. When I start to download a filelist and delete it from the queue before it finishes, the incomplete filelist remains on the disk. I think it should be deleted, just like other incomplete files that are removed from the queue. I'm using v0.668.

I've submitted this to bugzilla: http://dcplusplus.sourceforge.net/cgi-bin/bugzilla/show_bug.cgi?id=403.

[paka]

I wrote a workaround for the problem. I know it's horrible. But it works for me
QueueManager.cpp is the modified file. Here's the diff (Bugzilla seems to be down):

Code: Select all

@@ -847,10 +847,16 @@
 
 void QueueManager::remove(const string& aTarget) throw() {
    string x;
+   string tname = "";
    {
       Lock l(cs);
 
       QueueItem* q = fileQueue.find(aTarget);
+      if(q->isSet(QueueItem::FLAG_USER_LIST)) {
+         // In this function (remove), q->isSet(QueueItem::FLAG_XML_BZLIST) doesn't have a proper value,
+         // so q->getListName gives a wrong answer. - Paka
+         tname = q->getTarget();
+      }
       if(q != NULL) {
          if(q->isSet(QueueItem::FLAG_DIRECTORY_DOWNLOAD)) {
             dcassert(q->getSources().size() == 1);
@@ -878,6 +884,12 @@
    }
    if(!x.empty()) {
       DownloadManager::getInstance()->abortDownload(x);
+      if(tname != "") {
+         // Doesn't want to delete the file without the slowdown loop (why?)... - Paka
+         for(int64_t pyntla = 0; pyntla < 100000000; pyntla++) { };
+         File::deleteFile(tname + ".xml.bz2");
+         File::deleteFile(tname + ".DcLst");
+      }
    }
 }

I've sent this patch to Arne (just in case it helps somehow...).
Hopefully someone finds a better fix. I'd be glad to see one.

[GargoyleMT]

""? Use Util::emptyString instead. or tstring.empty().

What are the effects of changing getListName() instead?

[GargoyleMT]

// Doesn't want to delete the file without the slowdown loop (why?)... - Paka

Have you checked if the file is still open? =)

[paka]

Have you checked if the file is still open? =)

Most likely this is the problem. That's why I put that crappy slowdown loop I've tried to put the File::deleteFile call outside the remove function (specifically, in the piece of code where the event of Delete key pressed is handled), but the file remained on the disk. Do you have an idea how to delete the file properly?

Use Util::emptyString instead. or tstring.empty().

Thanks. I'm still learning the conventions that DC++ uses.

What are the effects of changing getListName() instead?

This leads to the same problem. Among the functions that handle the download, q->isSet(QueueItem::FLAG_XML_BZLIST) (used by getListName()) isn't passed. That's why I got .DcLst as an extension in getListName(), instead of .xml.bz2, which was the case. It was easier for me to use getTarget().

[GargoyleMT]

Most likely this is the problem.

Also, there's code to delete the lists already:

Code: Select all

               if(q->isSet(QueueItem::FLAG_USER_LIST)) {
                  // Blah...no use keeping an unfinished file list...
                  File::deleteFile(q->getListName());

in QueueManager::putDownload()

getListName() uses q->isSet(QueueItem::FLAG_XML_BZLIST) which is not set, because it isn't passed among the functions that handle the download.

If that's the case, something is probably going wrong in the comparison in QueueManager::putDownload()...

[paka]

Also, there's code to delete the lists already in QueueManager::putDownload()

Yes. AFAIK, this piece deletes the filelist when the transfer of a filelist was stopped, but the item remains in the queue. I've tried to copy it directly to the remove function, but in vain (even when I specified the filename directly). The filelist file in the remove function must have been open for some reason.

If that's the case, something is probably going wrong in the comparison in QueueManager::putDownload()...

QueueManager::putDownload gets the information from a Download*-type variable, from the name of the filelist:

Code: Select all

            if(aDownload->getSource() == "files.xml.bz2") {
               q->setFlag(QueueItem::FLAG_XML_BZLIST);
            } else {
               q->unsetFlag(QueueItem::FLAG_XML_BZLIST);
            }

and QueueManager::remove gets a string&-type aTarget parameter. The solutions that I see are:
- passing the XML_BZLIST flag among the download functions, from the moment the file is being opened (not very clever) or
- storing the flag in the fileQueue (this would be better probably).

[GargoyleMT]

Yes. AFAIK, this piece deletes the filelist when the transfer of a filelist was stopped, but the item remains in the queue. I've tried to copy it directly to the remove function, but in vain (even when I specified the filename directly). The filelist file in the remove function must have been open for some reason.

If you're removing file lists from the queue, and you want to remove the partials... Well, it seems that adding a call to QueueManager::putDownload() to DownloadManager::abortDownload() may be a good avenue to explore, too. That looks like how a running download is cancelled. Hmm, or abstracting the delete-partial code to another function.

[paka]

I've modified DownloadManager.cpp with this:

Code: Select all

@@ -905,6 +905,7 @@
       if(d->getTarget() == aTarget) {
          dcassert(d->getUserConnection() != NULL);
          d->getUserConnection()->disconnect();
+         QueueManager::getInstance()->putDownload(d, false);
          break;
       }
    }

(also tried some other places) and got this exception info:

Code: Select all

c:\dcpp\dcplusplus-0.668\client\downloadmanager.cpp(567): DownloadManager::on
c:\dcpp\dcplusplus-0.668\client\speaker.h(65): Speaker<UserConnectionListener>::fire<UserConnectionListener::X<2>=0x01CEFA00,UserConnection *=0x01CEF930,unsigned char *=0x01CEFA1C,unsigned int>
c:\dcpp\dcplusplus-0.668\client\userconnection.h(376): UserConnection::on

[GargoyleMT]

Hmm. Well, another off-the-cuff suggestion woudl be to move the abortDownload() in QueueManager::remove() up, before the item gets removed from the queue, near where the existing incompletes are deleted.

[paka]

I've modified QueueManager.cpp with this:

Code: Select all

@@ -861,6 +861,8 @@
             directories.erase(q->getSources()[0]->getUser());
          }
 
+         DownloadManager::getInstance()->abortDownload(q->getTarget());
+
          if(q->getStatus() == QueueItem::STATUS_RUNNING) {
             x = q->getTarget();
          } else if(!q->getTempTarget().empty() && q->getTempTarget() != q->getTarget()) {

and also put (in the second test) abortDownload after the end of "if(q->getStatus() == QueueItem::STATUS_RUNNING)", but the file remains on the disk. In both cases I was using the last lines of my patch that delete .DcLst and .xml.bz2 file, but removed the slowdown loop to check if putting abortDownload in other places will do the job.