Skip to content

Croak when fread,flock,lockf fail #14

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Flimm wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Croak when fread,flock,lockf fail #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Changes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
Revision history for Perl extension Data::UUID.

AS OF YET UNRELEASED
- exceptions are thrown when flock/lockf/fread fails

1.220 2014-12-15
- improve chances it'll work on Android (thanks, Brian Fraser)

Expand Down
12 changes: 6 additions & 6 deletions UUID.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,14 @@ typedef unsigned64_t perl_uuid_time_t;

/* Android's lic provides neither lockf nor any of the related constants */
#if (defined __solaris__ || defined __linux__) && !defined(__android__)
# define LOCK(f) lockf(fileno(f),F_LOCK,0);
# define UNLOCK(f) lockf(fileno(f),F_ULOCK,0);
# define LOCK(f) lockf(fileno(f),F_LOCK,0)
# define UNLOCK(f) lockf(fileno(f),F_ULOCK,0)
#elif defined __darwin__
# define LOCK(f) flock(fileno(f),LOCK_EX);
# define UNLOCK(f) flock(fileno(f),LOCK_UN);
# define LOCK(f) flock(fileno(f),LOCK_EX)
# define UNLOCK(f) flock(fileno(f),LOCK_UN)
#else
# define LOCK(f)
# define UNLOCK(f)
# define LOCK(f) 0
# define UNLOCK(f) 0
#endif

#undef perl_uuid_t
Expand Down
27 changes: 20 additions & 7 deletions UUID.xs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,16 +357,19 @@ PREINIT:
CODE:
RETVAL = (uuid_context_t *)PerlMemShared_malloc(sizeof(uuid_context_t));
if ((fd = fopen(UUID_STATE_NV_STORE, "rb"))) {
fread(&(RETVAL->state), sizeof(uuid_state_t), 1, fd);
if (! fread(&(RETVAL->state), sizeof(uuid_state_t), 1, fd)) {
croak("fread failed");
}
fclose(fd);
Copy link

@wolfsage wolfsage Jan 17, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calling croak(...) here leaves the fd open and leaks the file handle.

To see this in action on linux:

mkdir /tmp/.UUID_STATE
perl -Iblib/lib -Iblib/arch -MData::UUID -e 'eval { Data::UUID->new } for 1..100; <>;'

And in another terminal:

lsof | grep .UUID_STATE

We should fclose(fd) before calling croak every time that fd is already open.

Thanks.

get_current_time(&timestamp);
RETVAL->next_save = timestamp;
}
if ((fd = fopen(UUID_NODEID_NV_STORE, "rb"))) {
pid_t *hate = (pid_t *) &(RETVAL->nodeid);
fread(&(RETVAL->nodeid), sizeof(uuid_node_t), 1, fd );
if (! fread(&(RETVAL->nodeid), sizeof(uuid_node_t), 1, fd )) {
croak("fread failed");
}
fclose(fd);

*hate += getpid();
} else {
get_random_info(seed);
Expand Down Expand Up @@ -418,9 +421,13 @@ PPCODE:
if (timestamp > self->next_save ) {
mask = umask(_DEFAULT_UMASK);
if((fd = fopen(UUID_STATE_NV_STORE, "wb"))) {
LOCK(fd);
if (-1 == LOCK(fd)) {
croak("Could not lock");
}
fwrite(&(self->state), sizeof(uuid_state_t), 1, fd);
UNLOCK(fd);
if (-1 == UNLOCK(fd)) {
croak("Could not unlock");
}
fclose(fd);
}
umask(mask);
Expand Down Expand Up @@ -585,9 +592,15 @@ CODE:
if (count == 0) {
#endif
if ((fd = fopen(UUID_STATE_NV_STORE, "wb"))) {
LOCK(fd);
if (-1 == LOCK(fd)) {
PerlMemShared_free(self);
croak("Could not lock");
}
fwrite(&(self->state), sizeof(uuid_state_t), 1, fd);
UNLOCK(fd);
if (-1 == UNLOCK(fd)) {
PerlMemShared_free(self);
croak("Could not unlock");
}
fclose(fd);
};
PerlMemShared_free(self);
Expand Down