[ http://jira.qos.ch/browse/LBCORE-67?page=com.atlassian.jira.plugin.system.is... ] Joern Huxhorn updated LBCORE-67: -------------------------------- Attachment: LockPatchWithTest.patch Ok, I implemented to deadlock-tests. While it's not a realistic test the same situation could definitely happen in production in case of OutOfMemoryError as I wrote in the comment of the test. It's generally a very bad idea to save a bit of standard code just because a possible problem isn't imaginable at the moment. I'm not even sure if the code without try {} finally {} will execute faster or if there won't be any difference at all. This is one of the cases where I'll really welcome closures in java because those will eliminate this shortcoming, i.e. the manual handling of locks. In the future, code will look like this: lock(readLock) { // do something } instead of readLock.lock(); try { // do something } finally { readLock.unlock(); } but in the meantime we'll just have to stick to conventions.
Unsecure usage of locks in AppenderAttachableImpl -------------------------------------------------
Key: LBCORE-67 URL: http://jira.qos.ch/browse/LBCORE-67 Project: logback-core Issue Type: Bug Components: Appender Affects Versions: 0.9.10 Reporter: Joern Huxhorn Assignee: Logback dev list Attachments: LockPatch.patch, LockPatchWithTest.patch
The unlock of a lock should, I would even say "must", always be done in a finally block. Otherwise really bad things (deadlock) can happen if an exception is thrown. See http://java.sun.com/j2se/1.5.0/docs/api/java/util/concurrent/locks/Lock.html
-- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.qos.ch/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira