Peter Eisentraut <e99re41@DoCS.UU.SE> writes:
>> After further looking, I notice that users.c is one of the few places
>> that will drop AccessExclusiveLock at heap_close time rather than
>> holding it till xact commit. I wonder whether this is a bug...
> Well it was you who did it while introducing that second argument in the
> first place. I think the safest thing to do is definitely to hold any lock
> until transaction end. I'm not sure, shouldn't the transaction isolation
> level apply here as well? In the end, is there ever a good reason for
> releasing a lock in heap_close?
I provided that hook because of stuff like the LISTEN/NOTIFY support,
which grabs and releases locks on pg_listener --- as it's presently
designed, not releasing the lock at end of statement could hang the
entire system until you commit your transaction. (Probably
LISTEN/NOTIFY should be re-examined to see if it couldn't use a less
drastic lock than AccessExclusiveLock. That code hasn't been gone over
since before MVCC.) Also, I still think that in general it's OK to
release a read lock on a system table early. Write locks, maybe not.
>> I also notice that there definitely is a glaring bug there:
>> write_password_file() leaks one kernel file descriptor each time it runs
>> (note the creat() call).
> Wow, this has been there for over two years.
Yeah, a long time :-(. It's not the first resource leak we've found
in the password-related code, too. I wonder if there are more...
regards, tom lane