Opened 10 years ago

Closed 10 years ago

#4153 closed bug (fixed)

Installer doesn't stop collecting copy information when user pressed stop button

Reported by: idefix Owned by: stippi
Priority: normal Milestone: R1
Component: Applications/Installer Version: R1/pre-alpha1
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

When you press the stop button while Installer is collecting the copy information, it won't stop until it's finished collecting.

It should immediately stop, just like it does when copying files.

Steps to reproduce:

  • Press the Begin button; Installer begins collecting copy information
  • Press the Stop button; a windows pops up asking if you're sure
  • Press that window's Stop button
  • You'll notice that Installer continues collecting copy information. Pressing the Stop button again has no effect and you'll have to wait for Installer to finish collecting before it gets back to the main screen.

Change History (6)

comment:1 Changed 10 years ago by stippi

If you want to have a look, the stopping is done via a BLocker, IIRC. What the CopyEngine does with regards to this lock during the actual copy process needs to be replicated to the recursive collection of information. The problem may be that this slows down the collection somewhat, which ideally is fast as possible. Usually, users want to install, not stop, so one may implement a fix which degrades the usual case. I am undecided. Maybe the check for the lock wouldn't be so bad, since usually the file data is not in the cache, so the waiting times for the I/O are much worse than the check for the lock everywhere. It's probably ok to check only when entering a new directory, not for each file.

comment:2 in reply to:  1 Changed 10 years ago by idefix

Replying to stippi:

If you want to have a look, the stopping is done via a BLocker, IIRC. What the CopyEngine does with regards to this lock during the actual copy process needs to be replicated to the recursive collection of information.

Yes, I got that far by reading the source code. Unfortunately, at the moment, I don't understand enough of BLocker to implement it in the collecting process.

comment:3 Changed 10 years ago by stippi

The trick is that it's safe to call Lock() on a deleted BLocker. In that case, Lock() fails. So these lines:

AutoLocker<BLocker> lock(locker);
if (locker != NULL && !lock.IsLocked()) {
	// We are supposed to quit
	return B_CANCELED;
}

Make the copy process stop. You could just add this before entering a new directory.

comment:4 Changed 10 years ago by stippi

Owner: changed from korli to stippi

Actually, forget that. It's of course not safe to call Lock() on a deleted BLocker. I must be getting old, and Installer is broken in this respect!

comment:5 Changed 10 years ago by stippi

I think what should happen is that the BLocker should be replaced by a semaphore ID, though the mechanism would be absolutely similar.

comment:6 Changed 10 years ago by stippi

Resolution: fixed
Status: newclosed

Fixed in hrev31912. Turns out I wasn't quite so senile... the previous mechanism was based on the idea that you could delete a *locked* BLocker, which would make a thread which blocked on the locked BLocker get B_BAD_SEM as result for it's locking request. The new mechanism is more straight forward and doesn't have a race condition that the previous code needed to take into account.

Note: See TracTickets for help on using tickets.