Create Task
Maniphest T7824

[libzip] Check kill operation for each type of job
Closed, ResolvedPublic
Actions

Description

The libzip plugin doesn't react to QThread::requestInterruption(), as bug #389290 shows.

This happens because zip_close() (where we actually write data to disk) is blocking and we cannot check QThread::currentThread()->isInterruptionRequested() like we usually do.

This probably affects also other types of job (delete, move, etc.), so we need to check all of them.

Related Objects

elvisangelaccio created this task.Jan 28 2018, 1:54 PM
elvisangelaccio triaged this task as High priority.
elvisangelaccio added a comment.Jan 28 2018, 2:05 PM

Possible solution: call zip_discard(m_archive) in LibzipPlugin::doKill() (where m_archive stores the zip_t* returned by zip_open()).
Howewer this needs a mutex, since we are freeing m_archive from the main thread (while it was created in the secondary thread).

elvisangelaccio added a comment.EditedFeb 24 2018, 7:16 PM
In T7824#125217, @elvisangelaccio wrote:

Possible solution: call zip_discard(m_archive) in LibzipPlugin::doKill() (where m_archive stores the zip_t* returned by zip_open()).
Howewer this needs a mutex, since we are freeing m_archive from the main thread (while it was created in the secondary thread).

Not possible for the same reason: zip_close() is blocking, so LibzipPlugin::doKill() will always find the mutex locked... :/

(and without mutex, it will crash)

elvisangelaccio added a comment.Feb 24 2018, 11:08 PM

The test does can react to QThread::requestInterruption(), instead. It just doesn't currently do it.

elvisangelaccio moved this task from Backlog to In progress on the Ark board.Feb 25 2018, 12:37 PM
elvisangelaccio closed this task as Resolved.Oct 27 2018, 4:21 PM
elvisangelaccio claimed this task.
In T7824#130228, @elvisangelaccio wrote:

The test does can react to QThread::requestInterruption(), instead. It just doesn't currently do it.

Done with 0fba00d0186af24252754dc625bb2ae1b79f67e5