File-renaming causes "QObject::startTimer: timers cannot be started from another thread" error

[rb07]

Since r13810, and testing with r13863, file renaming only works partially: the file gets renamed, but the application stops refreshing the UI (for all torrents, and the opened details panel so you never see the change), and becomes unusable until you restart it (then it shows the changed file name).

My guess is that the call-back is the one starting the timer in a different thread.

session.cc:

Session :: localSessionCallback( tr_session * session, struct evbuffer * json, void * vself )
{
  Q_UNUSED (session);

  Session * self = static_cast<Session*>(vself);

  self->myIdleJSON.append (QString ((const char*) evbuffer_pullup (json, -1)));

  if (!self->myResponseTimer.isActive())
    self->myResponseTimer.start(50);
}

The error message in the summary is only shown in the console.

[jordan]

Interesting, I don't get that error message in the console whether I'm running a local or remote session. I wonder if it's platform-dependent.

Looking at ​http://cdumez.blogspot.com/2011/03/worker-thread-in-qt-using-signals-slots.html and ​http://stackoverflow.com/questions/6732583/qobjectstarttimer-timers-cannot-be-started-from-another-thread-without-time, it looks like a safer way to delegate work from the libtransmission thread to the Qt thread would be with signals/slots.

[jordan]

rb07, in r13864 I've replaced the list-and-timer mechanism with signal-and-slot to push the response from the libtransmission thread to the Qt thread.

I can confirm that this new approach works for me.

However, since I didn't get the console message on the previous approach, I'd like you to give this code a try before we mark this ticket as 'fixed'.

[rb07]

Replying to jordan:

However, since I didn't get the console message on the previous approach, I'd like you to give this code a try before we mark this ticket as 'fixed'.

Yes, it does fix the problem.

Thanks!

Side questions:

1. I had to disable the keyboard shortcuts in file-tree.cc :

       // handle using the keyboard to toggle the
       // wanted/unwanted state or the file priority
   /*** DISABLED: its incompatible with renaming files.
       else if( event->type() == QEvent::KeyPress )
       {
           switch( dynamic_cast<QKeyEvent*>(event)->key() )
           {
               case Qt::Key_Space:
                   foreach( QModelIndex i, selectionModel()->selectedRows(COL_WANTED) )
                       clicked( i );
                   return false;
   
               case Qt::Key_Enter:
               case Qt::Key_Return:
                   foreach( QModelIndex i, selectionModel()->selectedRows(COL_PRIORITY) )
                       clicked( i );
                   return false;
           }
       }
   ***/
   

which obviously interfere with editing the file name. Do you intend to keep them?

2. The file editing is also enabled in the (Open Torrent) options panel, where it doesn't work. Should editing be disabled there? or it should be working.

3. Probably specific to my Windows port, but renaming is (or was) also not working on a file being downloaded, I get "access denied". Is this the case on Linux? (then I probably need to fix the flags so that an open file can be renamed). Actually this one just changed to "file doesn't exist", the checking of .part may be off (r13865 and local session).

[jordan]

rb07, I think I've got items 1 and 2 fixed in r13893... please reopen this ticket if you see any remaining issues.

I'm not seeing 3 on my machine. :/

[rb07]

New issue w/r13894: Can't edit file names while downloading, the cursor keeps jumping to the end (every 3 or 4 sec) after replacing the string with the original. This (maybe) wasn't the case with the code from 2 days ago.

Another: Opening the torrent properties I get 2 identical lines in the console saying "I don't have this index".

Another: Something weird is going on while changing speed limits with the status bar drop down. The first click over a value doesn't do anything, or to be more precise, looks like it makes everything flicker, and doesn't change, or even select the speed. The second time it works, but if you close the drop down and open it again, the same thing happens.

[jordan]

gaah, I think I know what's causing the jumping cursor. Thanks for the heads-up rb07.

That last paragraph sounds unrelated? You may want to file a separate ticket for that, as much as it pains me to say so because I'm trying to get the bug count down under 200... :)

[jordan]

rb07, the flicker & resetting value should be fixed now in r13930. Could you test & see if it works for you on Windows as well?

[rb07]

Replying to jordan:

rb07, the flicker & resetting value should be fixed now in r13930. Could you test & see if it works for you on Windows as well?

Yes, the editing works fine now.

But changing the name while the torrent is downloading doesn't. I'm debugging to see what the actual problem is.

[rb07]

Yep, its a Windows problem, I'm getting a:

ERROR_SHARING_VIOLATION

    32 (0x20)

    The process cannot access the file because it is being used by another process.

Its just another thread dumb Windows. Oh well, time to test some Windows flags: FILE_SHARE_DELETE sounds about right...

[jordan]

rb07, any news on this?

[rb07]

Replying to jordan:

rb07, any news on this?

Yes, I just made it work on Windows.

Since my code has a lot of modifications (remember ticket #4160?), I don't think its worth adding a patch to this ticket. If anybody likes to see what the changes are like:

int tr_open (const char *pathUTF8, int oflag, ...)
{
    va_list a;
    int mode = 0;
    va_start(a, oflag);
    mode = va_arg(a, int);
    va_end(a);
#ifdef WIN32
    if (pathUTF8) {
        int n = strlen(pathUTF8) + 1;
        wchar_t pathUTF16[n];
        if (MultiByteToWideChar(CP_UTF8, 0, pathUTF8, -1, pathUTF16, n))
            /* binary mode is mandatory, otherwise UTF16 data is assumed */
#if 0
            return _wsopen(chkFilename(pathUTF16), oflag | _O_BINARY, _SH_DENYNO, mode);
#endif
        {   HANDLE h;
            h = CreateFileW(chkFilename(pathUTF16),
                            GENERIC_READ|GENERIC_WRITE,
                            FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
                            NULL,
                            OPEN_ALWAYS,
                            FILE_FLAG_RANDOM_ACCESS,
                            NULL);
            if (h == INVALID_HANDLE_VALUE)
            {   errno = GetLastError();
                return -1;
            }
            return _open_osfhandle(h, oflag | _O_BINARY);
        }
    }
    errno = ENOENT;
    return -1;
#else
    return open(pathUTF8, oflag, mode);
#endif
}

FILE *tr_fopen (const char *restrict filenameUTF8, const char *restrict mode)
{   
#ifdef WIN32
    if (filenameUTF8) {
        int n = strlen(filenameUTF8) + 1;
        int m = strlen(mode) + 2;
        wchar_t filenameUTF16[n];
        wchar_t modeUTF16[m];
        MultiByteToWideChar(CP_UTF8, 0, filenameUTF8, -1, filenameUTF16, n);
        MultiByteToWideChar(CP_UTF8, 0, mode, -1, modeUTF16, m);
        /* binary mode is mandatory, otherwise UTF16 data is assumed */
        if (!strchr(mode, 'b'))
            wcscat(modeUTF16, L"b");
#if 0
        return _wfsopen(chkFilename(filenameUTF16), modeUTF16, _SH_DENYNO);
#endif
        {   HANDLE h;
            h = CreateFileW(chkFilename(filenameUTF16),
                            GENERIC_READ|GENERIC_WRITE,
                            FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
                            NULL,
                            OPEN_ALWAYS,
                            FILE_FLAG_RANDOM_ACCESS,
                            NULL);
            if (h == INVALID_HANDLE_VALUE)
            {   errno = GetLastError();
                return NULL;
            }
            return _wfdopen(_open_osfhandle(h, _O_BINARY), modeUTF16);
        }
    }
    return NULL;
#else
    return fopen(filenameUTF8, mode);
#endif
}

I'm not sure if I'm adding more problems (since the "mode" in the open() function is not even used.)

[jordan]

Actually I had forgotten #4160. *shame*

I'd like to shrink the diffs between our two branches. Could you attach a refresh patched to #4160?

[rb07]

Replying to jordan:

Could you attach a refresh patched to #4160?

I don't think its worth the effort, my changes are not only about adding full support for foreign languages, I have added some 'features' (some proposed by other users in this tracker, some mine, some experimental, some unfinished), some bug fixes, and changes for using Qt5 (still unusable as of 5.0.1, it got worse w.r.t. plugins, and I use QtCurve style as a plugin).

Separating the different changes is a lot of work I don't want to do. So there is no clean patch for #4160.

[jordan]

fair enough.

Is there a public repo where I can see the changes that you've made? The latest changeset I could find was a patch against 2.72.

[rb07]

No, there is no public repository, I only use one which is my working copy of your Subversion repository. I don't keep track of my changes the way you do (yes I know, I shouldn't do that, its messy and bad practice).

I often forget to make a patch against released versions, that's why the last one I put on SF is against 2.72.

After the recent exchange in the forum ("your patches don't work", and "give me your code"), I'm rethinking the whole idea of making my code available (to people like that, or the other guy that gave you a patch, changing my code and breaking the way it worked on Windows probably on purpose because he didn't like my answers in the forum).

[jordan]

Despite the bad attitude of some forum users, there are several really strong reasons to make the code available:

• I'd like to merge parts of it into transmissionbt.com's repo, particularly portability patches. Ideally, the upstream code should be portable enough that downstream flavors can just add features without having to also fix issues like you're currently doing with remove(), rename(), et al.

• It's required by section 3a of the license.

• Transmission's openness is one of its biggest benefits to its users. If trqtw has unpublished patches, it opens itself to the same criticisms as other closed-source Windows clients. :(

[jordan]

rb07, also FWIW: ​https://forum.transmissionbt.com/viewtopic.php?p=63817#p63817

[jordan]

rb07, any chance you could attach a transmission-2.76-Qt-build.diff to https://trac.transmissionbt.com/wiki/BuildingTransmissionQtWindows ...?

[rb07]

No, I don't have it (as I said: I forgot to make the diff), and my 2.76 is not your 2.76 (i.e. I don't use the branch, I released based on the revision number -- which I didn't know Subversion duplicates, if I understand correctly, the rev. in head has the same number as one in the branch).

Anyway, if all you want is to see my current repository, then I can upload a diff to SF.

[jordan]

That would be great. :)

[rb07]

Done: ​http://sourceforge.net/p/trqtw/patches/12/

[jordan]

Thanks!

Moving past the discussion of code patches/repos... is there anything left to be done on this ticket?

[rb07]

No.

[rb07]

Replying to rb07:

Replying to jordan:

rb07, any news on this?

Yes, I just made it work on Windows.

For the record: No, I didn't.

Currently I can rename files at any time. But directories can only be renamed before they are created, and after download finishes.

The root of this (minor) problem is that mkdir in Windows doesn't have a way, or alternative function, to set share attributes.

I'm not sure if I'm adding more problems (since the "mode" in the open() function is not even used.)

I added mode handling.