Hopefully fixed a file descriptor resource leak in sg_grab()

This commit is contained in:
Thomas Schmitt 2006-08-20 19:03:21 +00:00
parent 2fa6787d14
commit b5bafeb048
3 changed files with 181 additions and 7 deletions

View File

@ -12,6 +12,20 @@
int burn_running = 0; int burn_running = 0;
/* ts A60813 : wether to use O_EXCL and/or O_NONBLOCK in libburn/sg.c */
int burn_sg_open_o_excl = 1;
/* O_NONBLOCK was hardcoded in enumerate_ata() which i hardly use.
For enumerate_sg() it seems ok.
So it should stay default mode until enumerate_ata() without O_NONBLOCK
has been thoroughly tested. */
int burn_sg_open_o_nonblock = 1;
/* wether to take a busy drive as an error */
/* Caution: this is implemented by a rough hack and eventually leads
to unconditional abort of the process */
int burn_sg_open_abort_busy = 0;
int burn_initialize(void) int burn_initialize(void)
{ {
if (burn_running) if (burn_running)
@ -31,3 +45,30 @@ void burn_finish(void)
burn_running = 0; burn_running = 0;
} }
/* ts A60813 */
/** Set parameters for behavior on opening device files. To be called early
after burn_initialize() and before any bus scan. But not mandatory at all.
@param exclusive Try to open only devices which are not marked as busy
and try to mark them busy if opened sucessfully. (O_EXCL)
There are kernels which simply don't care about O_EXCL.
Some have it off, some have it on, some are switchable.
@param blocking Try to wait for drives which do not open immediately but
also do not return an error as well. (O_NONBLOCK)
This might stall indefinitely with /dev/hdX hard disks.
@param abort_on_busy Unconditionally abort process when a non blocking
exclusive opening attempt indicates a busy drive.
Use this only after thorough tests with your app.
Parameter value 1 enables a feature, 0 disables.
Default is (1,0,0). Have a good reason before you change it.
*/
void burn_preset_device_open(int exclusive, int blocking, int abort_on_busy)
{
assert(burn_running);
burn_sg_open_o_excl= !!exclusive;
burn_sg_open_o_nonblock= !blocking;
burn_sg_open_abort_busy= !!abort_on_busy;
}

View File

@ -442,6 +442,24 @@ void burn_finish(void);
*/ */
void burn_set_verbosity(int level); void burn_set_verbosity(int level);
/* ts A60813 */
/** Set parameters for behavior on opening device files. To be called early
after burn_initialize() and before any bus scan. But not mandatory at all.
Parameter value 1 enables a feature, 0 disables.
Default is (1,0,0). Have a good reason before you change it.
@param exclusive Try to open only devices which are not marked as busy
and try to mark them busy if opened sucessfully. (O_EXCL)
There are kernels which simply don't care about O_EXCL.
Some have it off, some have it on, some are switchable.
@param blocking Try to wait for drives which do not open immediately but
also do not return an error as well. (O_NONBLOCK)
This might stall indefinitely with /dev/hdX hard disks.
@param abort_on_busy Unconditionally abort process when a non blocking
exclusive opening attempt indicates a busy drive.
Use this only after thorough tests with your app.
*/
void burn_preset_device_open(int exclusive, int blocking, int abort_on_busy);
/** Returns a newly allocated burn_message structure. This message should be /** Returns a newly allocated burn_message structure. This message should be
freed with burn_message_free() when you are finished with it. freed with burn_message_free() when you are finished with it.
@return A message or NULL when there are no more messages to retrieve. @return A message or NULL when there are no more messages to retrieve.

View File

@ -28,6 +28,15 @@ static void enumerate_common(char *fname);
/* ts A51221 */ /* ts A51221 */
int burn_drive_is_banned(char *device_address); int burn_drive_is_banned(char *device_address);
/* ts A60813 : storage objects are in libburn/init.c
wether to use O_EXCL
wether to use O_NOBLOCK with open(2) on devices
wether to take O_EXCL rejection as fatal error */
extern int burn_sg_open_o_excl;
extern int burn_sg_open_o_nonblock;
extern int burn_sg_open_abort_busy;
static int sgio_test(int fd) static int sgio_test(int fd)
{ {
unsigned char test_ops[] = { 0, 0, 0, 0, 0, 0 }; unsigned char test_ops[] = { 0, 0, 0, 0, 0, 0 };
@ -48,6 +57,24 @@ void ata_enumerate(void)
int i, fd; int i, fd;
char fname[10]; char fname[10];
/* ts A60813 */
int open_mode = O_RDWR;
/* ts A60813
O_EXCL with block devices is an unpublished feature
of Linux kernels. Possibly introduced 2002.
It can only be used if libburn stops opening several
file descriptor on the same block device.
See comment in sg_grab() */
if(burn_sg_open_o_excl)
open_mode |= O_EXCL;
/* ts A60813
O_NONBLOCK was already hardcoded in ata_ but not in sg_.
There must be some reason for this. So O_NONBLOCK is
default mode for both now. Disable on own risk. */
if(burn_sg_open_o_nonblock)
open_mode |= O_NONBLOCK;
for (i = 0; i < 26; i++) { for (i = 0; i < 26; i++) {
sprintf(fname, "/dev/hd%c", 'a' + i); sprintf(fname, "/dev/hd%c", 'a' + i);
/* open O_RDWR so we don't think read only drives are /* open O_RDWR so we don't think read only drives are
@ -56,10 +83,23 @@ void ata_enumerate(void)
/* ts A51221 */ /* ts A51221 */
if (burn_drive_is_banned(fname)) if (burn_drive_is_banned(fname))
continue; continue;
fd = open(fname, O_RDWR | O_NONBLOCK); fd = open(fname, open_mode);
if (fd == -1) if (fd == -1) {
/* <<< debugging
fprintf(stderr,
"\nlibburn: experimental: fname= %s , errno= %d\n",
fname,errno);
*/
/* ts A60814 : i see no way to do this more nicely */
if (errno == EBUSY && burn_sg_open_abort_busy) {
fprintf(stderr,
"\nlibburn: FATAL : Application triggered abort on busy drive '%s'\n",
fname);
/* <<< maybe one should plainly exit here */
assert("drive busy" == "non fatal");
}
continue; continue;
}
/* found a drive */ /* found a drive */
ioctl(fd, HDIO_GET_IDENTITY, &tm); ioctl(fd, HDIO_GET_IDENTITY, &tm);
@ -86,6 +126,34 @@ void sg_enumerate(void)
int i, fd; int i, fd;
char fname[10]; char fname[10];
/* ts A60813 */
int open_mode = O_RDWR;
/* ts A60813
O_EXCL with block devices is an unpublished feature
of Linux kernels. Possibly introduced 2002.
It can only be used if libburn stops opening several
file descriptor on the same block device.
See comment in sg_grab() */
if(burn_sg_open_o_excl)
open_mode |= O_EXCL;
/* ts A60813
O_NONBLOCK was not hardcoded in sg_ but was in ata_.
I myself test mainly sg_ and it seems to be ok with
O_NONBLOCK. So it should stay default mode. */
if(burn_sg_open_o_nonblock)
open_mode |= O_NONBLOCK;
fprintf(stderr,
"\nlibburn: experimental: o_excl= %d , o_nonblock= %d, abort_on_busy= %d\n",
burn_sg_open_o_excl,burn_sg_open_o_nonblock,burn_sg_open_abort_busy);
fprintf(stderr,
"libburn: experimental: O_EXCL= %d , O_NONBLOCK= %d\n",
!!(open_mode&O_EXCL),!!(open_mode&O_NONBLOCK));
/* <<< debugging
*/
for (i = 0; i < 32; i++) { for (i = 0; i < 32; i++) {
sprintf(fname, "/dev/sg%d", i); sprintf(fname, "/dev/sg%d", i);
/* open RDWR so we don't accidentally think read only drives /* open RDWR so we don't accidentally think read only drives
@ -94,10 +162,24 @@ void sg_enumerate(void)
/* ts A51221 */ /* ts A51221 */
if (burn_drive_is_banned(fname)) if (burn_drive_is_banned(fname))
continue; continue;
fd = open(fname, O_RDWR); fd = open(fname, open_mode);
if (fd == -1)
continue;
if (fd == -1) {
/* <<< debugging
fprintf(stderr,
"\n cdrskin: experimental: fname= %s , errno= %d\n",
fname,errno);
*/
/* ts A60814 : i see no way to do this more nicely */
if (errno == EBUSY && burn_sg_open_abort_busy) {
fprintf(stderr,
"\nlibburn: FATAL : Application triggered abort on busy drive '%s'\n",
fname);
/* <<< maybe one should plainly exit here */
assert("drive busy" == "non fatal");
}
continue;
}
/* found a drive */ /* found a drive */
ioctl(fd, SG_GET_SCSI_ID, &sid); ioctl(fd, SG_GET_SCSI_ID, &sid);
close(fd); close(fd);
@ -167,14 +249,47 @@ static void enumerate_common(char *fname)
we use the sg reference count to decide whether we can use the we use the sg reference count to decide whether we can use the
drive or not. drive or not.
if refcount is not one, drive is open somewhere else. if refcount is not one, drive is open somewhere else.
ts A60813: this test is too late. O_EXCL is the stronger solution.
After all the test was diabled already in icculus.org/burn CVS.
*/ */
int sg_grab(struct burn_drive *d) int sg_grab(struct burn_drive *d)
{ {
int fd, count; int fd, count;
fd = open(d->devname, O_RDWR | O_NONBLOCK); /* ts A60813 */
int open_mode = O_RDWR;
/* ts A60813
O_EXCL with block devices is an unpublished feature
of Linux kernels. Possibly introduced 2002.
It can only be used if libburn stops opening several
file descriptor on the same block device.
See comment below */
if(burn_sg_open_o_excl)
open_mode |= O_EXCL;
/* ts A60813
O_NONBLOCK was hardcoded here. So it should stay default mode. */
if(burn_sg_open_o_nonblock)
open_mode |= O_NONBLOCK;
/* ts A60813
After enumeration the drive fd is probably still open.
-1337 is the initial value of burn_drive.fd and the value after
relase of drive. Unclear why not the official error return
value -1 of open(2) war used. */
if(d->fd == -1337)
fd = open(d->devname, open_mode);
else
fd= d->fd;
assert(fd != -1337); assert(fd != -1337);
if (-1 != fd) { if (-1 != fd) {
/* ts A60814:
according to my experiments this test would work now ! */
/* er = ioctl(fd, SG_GET_ACCESS_COUNT, &count);*/ /* er = ioctl(fd, SG_GET_ACCESS_COUNT, &count);*/
count = 1; count = 1;
if (1 == count) { if (1 == count) {