Re: K3b 1.0.1 eject problem

Sebastian Trüg <[email protected]>
Newsgroups gmane.comp.kde.k3b
Message-ID <[email protected]> 
On Thursday 21 June 2007 17:20:01 [email protected] wrote:
> I attach the K3b output (I used the patch: k3b-thread-lock-debug.diff with
> plain k3b code, DVD+R VIDEO, no eject)
>
> I didn't know what I should do with the sr0 device, so I didn't disconnect
> it.

Thanks a lot.
Well, at least I found a flaw in my implementation: the following can happen:

thread 1 opens device (K3bDevice::Device::open)
thread 2 opens device (K3bDevice::Device::open)
thread 1 closes device
thread 2 issues command (still thinks device is open) -> crash or at least 
failure

I fixed this with a combination of your patch and some additional changes that 
make sure that a thread always blocks a whole open-command-close block.
The downside is that now open and close are not entirely thread-safe anymore. 
It is possible to close the device while another thread is issuing a command.
I don't know how to solve that properly yet without closing the Device API (I 
think it can't be done).

Anyway, maybe this was the problem all along? I don't know. Could you please 
test the patch with clean k3b code since here it still works (stupid flawless 
system! ;)

Cheers,
Sebastian

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

_______________________________________________
K3b-user mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/k3b-user
k3b-thread.diff (text/x-diff, 13.7 KB) 
Index: libk3bdevice/k3bdevice.h
===================================================================
--- libk3bdevice/k3bdevice.h	(revision 665632)
+++ libk3bdevice/k3bdevice.h	(working copy)
@@ -40,7 +40,7 @@
    *
    * Devices are constructed by the DeviceManager.
    *
-   * All methods in Device are thread-safe which basicly means that
+   * All methods except for open and close in Device are thread-safe which basicly means that
    * no two commands are sent to the device at the same time.
    */
   // FIXME: all methods are const which makes no sense at all!
@@ -454,12 +454,16 @@
        * Open the device for access via a file descriptor.
        * @return true on success or if the device is already open.
        * @see close()
+       *
+       * Be aware that this method is not thread-safe.
        */
       bool open( bool write = false ) const;
 
       /**
        * Close the files descriptor.
        * @see open()
+       *
+       * Be aware that this method is not thread-safe.
        */
       void close() const;
 
@@ -700,6 +704,13 @@
       bool getNextWritableAdress( unsigned int& lastSessionStart, unsigned int& nextWritableAdress ) const;
 
       /**
+       * Retrieve the next writable address from the currently mounted writable medium.
+       * \return The next writable address if the medium is empty or appendable or -1
+       * if an error occured.
+       */
+      int nextWritableAddress() const;
+
+      /**
        * Locks the device for usage. This means that no MMC command can be performed
        * until usageUnlock is called.
        *
Index: libk3bdevice/k3bscsicommand_linux.cpp
===================================================================
--- libk3bdevice/k3bscsicommand_linux.cpp	(revision 665632)
+++ libk3bdevice/k3bscsicommand_linux.cpp	(working copy)
@@ -95,15 +95,20 @@
 {
   bool needToClose = false;
   if( m_device ) {
-    if( !m_device->isOpen() ) {
-      needToClose = true;
-    }
-    m_device->open( dir == TR_DIR_WRITE );
-    m_deviceHandle = m_device->handle();
+      m_device->usageLock();
+      if( !m_device->isOpen() ) {
+          needToClose = true;
+      }
+      if ( !m_device->open( dir == TR_DIR_WRITE ) ) {
+          m_device->usageUnlock();
+          return -1;
+      }
+      m_deviceHandle = m_device->handle();
   }
 
-  if( m_deviceHandle == -1 )
+  if( m_deviceHandle == -1 ) {
     return -1;
+  }
 
   int i = -1;
 
@@ -124,13 +129,7 @@
     else
       d->sgIo.dxfer_direction = SG_DXFER_NONE;
 
-    if ( m_device ) {
-        m_device->usageLock();
-    }
     i = ::ioctl( m_deviceHandle, SG_IO, &d->sgIo );
-    if ( m_device ) {
-        m_device->usageUnlock();
-    }
 
     if( ( d->sgIo.info&SG_INFO_OK_MASK ) != SG_INFO_OK )
       i = -1;
@@ -145,21 +144,19 @@
       d->cmd.data_direction = CGC_DATA_WRITE;
     else
       d->cmd.data_direction = CGC_DATA_NONE;
-    
-    if ( m_device ) {
-        m_device->usageLock();
-    }
+
     i = ::ioctl( m_deviceHandle, CDROM_SEND_PACKET, &d->cmd );
-    if ( m_device ) {
-        m_device->usageUnlock();
-    }
 #ifdef SG_IO
   }
-#endif    
+#endif
 
   if( needToClose )
     m_device->close();
-    
+
+  if ( m_device ) {
+      m_device->usageUnlock();
+  }
+
   if( i ) {
     debugError( d->cmd.cmd[0],
 		d->sense.error_code,
@@ -167,7 +164,7 @@
 		d->sense.asc,
 		d->sense.ascq );
 
-    int errCode = 
+    int errCode =
       (d->sense.error_code<<24) & 0xF000 |
       (d->sense.sense_key<<16)  & 0x0F00 |
       (d->sense.asc<<8)         & 0x00F0 |
Index: libk3bdevice/k3bdevice.cpp
===================================================================
--- libk3bdevice/k3bdevice.cpp	(revision 665632)
+++ libk3bdevice/k3bdevice.cpp	(working copy)
@@ -176,6 +176,7 @@
   bool burnfree;
 
   QMutex mutex;
+    QMutex openCloseMutex;
 };
 
 
@@ -1342,14 +1343,14 @@
   struct cdrom_tochdr tochdr;
   struct cdrom_tocentry tocentry;
 
+  usageLock();
   if( open() ) {
     //
     // CDROMREADTOCHDR ioctl returns:
     // cdth_trk0: First Track Number
     // cdth_trk1: Last Track Number
     //
-    usageLock();
-    if( ::ioctl(d->deviceFd,CDROMREADTOCHDR,&tochdr) ) {
+    if( ::ioctl( d->deviceFd, CDROMREADTOCHDR, &tochdr ) ) {
       k3bDebug() << "(K3bDevice::Device) could not get toc header !" << endl;
       success = false;
     }
@@ -1384,7 +1385,7 @@
         //                 2: CD-XA Mode2
         //
 
-        if( ::ioctl(d->deviceFd,CDROMREADTOCENTRY,&tocentry) ) {
+        if( ::ioctl( d->deviceFd, CDROMREADTOCENTRY, &tocentry ) ) {
           k3bDebug() << "(K3bDevice::Device) error reading tocentry " << i << endl;
 	  success = false;
 	  break;
@@ -1418,7 +1419,6 @@
         lastTrack = Track( startSec, startSec, trackType, trackMode );
       }
     }
-    usageUnlock();
 
     if( needToClose )
       close();
@@ -1426,6 +1426,8 @@
   else
     success = false;
 
+  usageUnlock();
+
   return success;
 }
 #endif // Q_OS_LINUX
@@ -1481,45 +1483,43 @@
   // So we use the ioctl on Linux systems
   //
 #if defined(Q_OS_LINUX)
-  bool needToClose = !isOpen();
-
-  if( open() ) {
-        usageLock();
-    bool success = ( ::ioctl( d->deviceFd, CDROM_LOCKDOOR, b ? 1 : 0 ) == 0 );
-        usageUnlock();
-    if( needToClose )
-      close();
-    return success;
-  }
-  else
-    return false;
+    bool success = false;
+    bool needToClose = !isOpen();
+    usageLock();
+    if( open() ) {
+        success = ( ::ioctl( d->deviceFd, CDROM_LOCKDOOR, b ? 1 : 0 ) == 0 );
+        if( needToClose )
+            close();
+    }
+    usageUnlock();
+    if ( success )
+        return success;
 #elif defined(Q_OS_NETBSD)
-  bool needToClose = !isOpen();
-  int arg = b ? 1 : 0;
+    bool success = false;
+    bool needToClose = !isOpen();
+    int arg = b ? 1 : 0;
+    usageLock();
+    if( open() ) {
+        success = ( ::ioctl( d->deviceFd, DIOCLOCK, &arg ) == 0 );
+        if( needToClose )
+            close();
+    }
+    usageUnlock();
+    if ( success )
+        return success;
+#endif
 
-  if( open() ) {
-        usageLock();
-    bool success = ( ::ioctl( d->deviceFd, DIOCLOCK, &arg ) == 0 );
-        usageUnlock();
-    if( needToClose )
-      close();
-    return success;
-  }
-  else
-    return false;
-#else
-  ScsiCommand cmd( this );
-  cmd[0] = MMC_PREVENT_ALLOW_MEDIUM_REMOVAL;
-  cmd[5] = 0; // Necessary to set the proper command length
-  if( b )
-    cmd[4] = 0x01;
-  int r = cmd.transport( TR_DIR_WRITE );
+    ScsiCommand cmd( this );
+    cmd[0] = MMC_PREVENT_ALLOW_MEDIUM_REMOVAL;
+    cmd[5] = 0; // Necessary to set the proper command length
+    if( b )
+        cmd[4] = 0x01;
+    int r = cmd.transport( TR_DIR_WRITE );
 
-  if( r )
-    k3bDebug() << "(K3bDevice::Device) MMC ALLOW MEDIA REMOVAL failed." << endl;
+    if( r )
+        k3bDebug() << "(K3bDevice::Device) MMC ALLOW MEDIA REMOVAL failed." << endl;
 
-  return ( r == 0 );
-#endif
+    return ( r == 0 );
 }
 
 bool K3bDevice::Device::rewritable() const
@@ -1543,90 +1543,90 @@
 bool K3bDevice::Device::eject() const
 {
 #ifdef Q_OS_NETBSD
-  bool success = false;
-  bool needToClose = !isOpen();
-  int arg = 0;
+    bool success = false;
+    bool needToClose = !isOpen();
+    int arg = 0;
 
-  if( open() ) {
-        usageLock();
-    if ( ::ioctl( d->deviceFd, DIOCEJECT, &arg ) >= 0)
-      success = true;
-        usageUnlock();
-    if( needToClose )
-      close();
-  }
-  if ( success )
-      return success;
+    usageLock();
+    if( open() ) {
+        if ( ::ioctl( d->deviceFd, DIOCEJECT, &arg ) >= 0)
+            success = true;
+        if( needToClose )
+            close();
+    }
+    usageUnlock();
+    if ( success )
+        return success;
 #elif defined(Q_OS_LINUX)
-  bool success = false;
-  bool needToClose = !isOpen();
+    bool success = false;
+    bool needToClose = !isOpen();
 
-  if( open() ) {
-        usageLock();
+    usageLock();
+    if( open() ) {
         if( ::ioctl( d->deviceFd, CDROMEJECT ) >= 0 )
-      success = true;
-        usageUnlock();
-    if( needToClose )
-      close();
-  }
-  if ( success )
-      return success;
+            success = true;
+        if( needToClose )
+            close();
+    }
+    usageUnlock();
+    if ( success )
+        return success;
 #endif
 
-  ScsiCommand cmd( this );
+    ScsiCommand cmd( this );
     cmd[0] = MMC_PREVENT_ALLOW_MEDIUM_REMOVAL;
-  cmd[5] = 0; // Necessary to set the proper command length
+    cmd[5] = 0; // Necessary to set the proper command length
     cmd.transport();
 
     cmd[0] = MMC_START_STOP_UNIT;
     cmd[5] = 0; // Necessary to set the proper command length
-  cmd[4] = 0x1;      // Start unit
-  cmd.transport();
+    cmd[4] = 0x1;      // Start unit
+    cmd.transport();
 
-  cmd[4] = 0x2;    // LoEj = 1, Start = 0
+    cmd[4] = 0x2;    // LoEj = 1, Start = 0
 
-  return !cmd.transport();
+    return !cmd.transport();
 }
 
 
 bool K3bDevice::Device::load() const
 {
 #ifdef Q_OS_NETBSD
-  bool success = false;
-  bool needToClose = !isOpen();
-  int arg = 0;
+    bool success = false;
+    bool needToClose = !isOpen();
+    int arg = 0;
 
-  if( open() ) {
-        usageLock();
-    if ( ::ioctl( d->deviceFd, CDIOCCLOSE, &arg ) >= 0)
-      success = true;
-        usageUnlock();
-    if( needToClose )
-      close();
-  }
-  if ( success )
-      return success;
+    usageLock();
+    if( open() ) {
+        if ( ::ioctl( d->deviceFd, CDIOCCLOSE, &arg ) >= 0)
+            success = true;
+        if( needToClose )
+            close();
+    }
+    usageUnlock();
+    if ( success )
+        return success;
 #elif defined(Q_OS_LINUX)
-  bool success = false;
-  bool needToClose = !isOpen();
+    bool success = false;
+    bool needToClose = !isOpen();
 
-  if( open() ) {
-        usageLock();
+    usageLock();
+    if( open() ) {
         if( ::ioctl( d->deviceFd, CDROMCLOSETRAY ) >= 0 )
-      success = true;
-        usageUnlock();
-    if( needToClose )
-      close();
-  }
-  if ( success )
-      return success;
+            success = true;
+        if( needToClose )
+            close();
+    }
+    usageUnlock();
+    if ( success )
+        return success;
 #endif
 
-  ScsiCommand cmd( this );
-  cmd[0] = MMC_START_STOP_UNIT;
-  cmd[4] = 0x3;    // LoEj = 1, Start = 1
-  cmd[5] = 0;      // Necessary to set the proper command length
-  return !cmd.transport();
+    ScsiCommand cmd( this );
+    cmd[0] = MMC_START_STOP_UNIT;
+    cmd[4] = 0x3;    // LoEj = 1, Start = 1
+    cmd[5] = 0;      // Necessary to set the proper command length
+    return !cmd.transport();
 }
 
 
@@ -1636,14 +1636,14 @@
 #ifdef Q_OS_LINUX
 
   bool needToClose = !isOpen();
+  usageLock();
   if ( open() ) {
-      usageLock();
       success = ( ::ioctl( d->deviceFd, CDROMEJECT_SW, enabled ? 1 : 0 ) == 0 );
-      usageUnlock();
       if ( needToClose ) {
           close();
       }
   }
+  usageUnlock();
 #endif
   return success;
 }
@@ -1677,7 +1677,7 @@
   if( d->openedReadWrite != write )
     close();
 
-  d->mutex.lock();
+  QMutexLocker ml( &d->openCloseMutex );
 
   d->openedReadWrite = write;
 
@@ -1688,16 +1688,12 @@
 	      << ((d->cam)?" succeeded.":" failed.") << endl;
   }
 
-  d->mutex.unlock();
-
   return (d->cam != 0);
 #endif
 #if defined(Q_OS_LINUX) || defined(Q_OS_NETBSD)
   if( d->deviceFd == -1 )
     d->deviceFd = openDevice( QFile::encodeName(devicename()), write );
 
-  d->mutex.unlock();
-
   return ( d->deviceFd != -1 );
 #endif
 }
@@ -1705,7 +1701,7 @@
 
 void K3bDevice::Device::close() const
 {
-  d->mutex.lock();
+  QMutexLocker ml( &d->openCloseMutex );
 
 #ifdef Q_OS_FREEBSD
   if( d->cam ) {
@@ -1719,8 +1715,6 @@
     d->deviceFd = -1;
   }
 #endif
-
-  d->mutex.unlock();
 }
 
 
@@ -2200,10 +2194,18 @@
 	K3b::Msf currentMax;
 	int currentMaxFormat = 0;
 	if( readFormatCapacity( 0x26, inf.m_capacity, &currentMax, &currentMaxFormat ) ) {
-	  if( inf.bgFormatState() != BG_FORMAT_NONE )
-	    inf.m_usedCapacity = currentMax;
-	  else
-	    inf.m_usedCapacity = 0;
+            if( currentMaxFormat == 0x1 ) { // unformatted or blank media
+                inf.m_usedCapacity = 0;
+                inf.m_capacity = currentMax;
+            }
+            else {
+                inf.m_usedCapacity = currentMax;
+                // Plextor drives tend to screw things up and report invalid values
+                // for the max format capacity of 1.4 GB DVD media
+                if ( inf.bgFormatState() == BG_FORMAT_COMPLETE ) {
+                    inf.m_capacity = currentMax;
+                }
+            }
        	}
 	else
 	  k3bDebug() << "(K3bDevice::Device) " << blockDeviceName()
@@ -3535,6 +3537,46 @@
 }
 
 
+int K3bDevice::Device::nextWritableAddress() const
+{
+    unsigned char* data = 0;
+    unsigned int dataLen = 0;
+    int nwa = -1;
+
+    if( readDiscInformation( &data, dataLen ) ) {
+        disc_info_t* inf = (disc_info_t*)data;
+
+        //
+        // The state of the last session has to be "empty" (0x0) or "incomplete" (0x1)
+        // The procedure here is taken from the dvd+rw-tools and wodim
+        //
+        if( !(inf->border & 0x2) ) {
+            // the incomplete track number is the first track in the last session (the empty session)
+            int nextTrack = inf->first_track_l|inf->first_track_m<<8;
+
+            unsigned char* trackData = 0;
+            unsigned int trackDataLen = 0;
+
+            // Read start address of the incomplete track
+            if( readTrackInformation( &trackData, trackDataLen, 0x1, nextTrack ) ) {
+                nwa = from4Byte( &trackData[8] );
+                delete [] trackData;
+            }
+
+            // Read start address of the invisible track
+            else if ( readTrackInformation( &trackData, trackDataLen, 0x1, 0xff ) ) {
+                nwa = from4Byte( &trackData[8] );
+                delete [] trackData;
+            }
+        }
+
+        delete [] data;
+    }
+
+    return nwa;
+}
+
+
 QCString K3bDevice::Device::mediaId( int mediaType ) const
 {
   QCString id;
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.