From: Jiří Zárevúcky Date: Wed, 24 Mar 2010 07:36:37 +0000 (+0100) Subject: Fix lock statement X-Git-Tag: 0.8.0~54 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=40c1dbfbfedb6c4a6b88df045eb1c2e7bdd38d93;p=thirdparty%2Fvala.git Fix lock statement This patch converts lock statements into try finally statements to ensure that unlock is always called. Fixes bug 582553. --- diff --git a/codegen/valaccodebasemodule.vala b/codegen/valaccodebasemodule.vala index 89ef3d583..223d49a21 100644 --- a/codegen/valaccodebasemodule.vala +++ b/codegen/valaccodebasemodule.vala @@ -3400,28 +3400,26 @@ internal class Vala.CCodeBaseModule : CCodeModule { return "__lock_%s".printf (symname); } - public override void visit_lock_statement (LockStatement stmt) { - var cn = new CCodeFragment (); + private CCodeExpression get_lock_expression (Statement stmt, Expression resource) { CCodeExpression l = null; - CCodeFunctionCall fc; - var inner_node = ((MemberAccess)stmt.resource).inner; - var member = (Member)stmt.resource.symbol_reference; - var parent = (TypeSymbol) stmt.resource.symbol_reference.parent_symbol; + var inner_node = ((MemberAccess)resource).inner; + var member = (Member)resource.symbol_reference; + var parent = (TypeSymbol)resource.symbol_reference.parent_symbol; if (member.is_instance_member ()) { if (inner_node == null) { l = new CCodeIdentifier ("self"); - } else if (stmt.resource.symbol_reference.parent_symbol != current_type_symbol) { - l = generate_instance_cast ((CCodeExpression) inner_node.ccodenode, parent); + } else if (resource.symbol_reference.parent_symbol != current_type_symbol) { + l = generate_instance_cast ((CCodeExpression) inner_node.ccodenode, parent); } else { l = (CCodeExpression) inner_node.ccodenode; } - l = new CCodeMemberAccess.pointer (new CCodeMemberAccess.pointer (l, "priv"), get_symbol_lock_name (stmt.resource.symbol_reference.name)); + l = new CCodeMemberAccess.pointer (new CCodeMemberAccess.pointer (l, "priv"), get_symbol_lock_name (resource.symbol_reference.name)); } else if (member.is_class_member ()) { CCodeExpression klass; - if (current_method != null && current_method.binding == MemberBinding.INSTANCE || + if (current_method != null && current_method.binding == MemberBinding.INSTANCE || current_property_accessor != null && current_property_accessor.prop.binding == MemberBinding.INSTANCE || (in_constructor && !in_static_or_class_context)) { var k = new CCodeFunctionCall (new CCodeIdentifier ("G_OBJECT_GET_CLASS")); @@ -3433,23 +3431,33 @@ internal class Vala.CCodeBaseModule : CCodeModule { var get_class_private_call = new CCodeFunctionCall (new CCodeIdentifier ("%s_GET_CLASS_PRIVATE".printf(parent.get_upper_case_cname ()))); get_class_private_call.add_argument (klass); - l = new CCodeMemberAccess.pointer (get_class_private_call, get_symbol_lock_name (stmt.resource.symbol_reference.name)); + l = new CCodeMemberAccess.pointer (get_class_private_call, get_symbol_lock_name (resource.symbol_reference.name)); } else { - string lock_name = "%s_%s".printf(parent.get_lower_case_cname (), stmt.resource.symbol_reference.name); + string lock_name = "%s_%s".printf(parent.get_lower_case_cname (), resource.symbol_reference.name); l = new CCodeIdentifier (get_symbol_lock_name (lock_name)); } + return l; + } - fc = new CCodeFunctionCall (new CCodeIdentifier (((Method) mutex_type.scope.lookup ("lock")).get_cname ())); + public override void visit_lock_statement (LockStatement stmt) { + var l = get_lock_expression (stmt, stmt.resource); + + var fc = new CCodeFunctionCall (new CCodeIdentifier (((Method) mutex_type.scope.lookup ("lock")).get_cname ())); fc.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, l)); + var cn = new CCodeFragment (); cn.append (new CCodeExpressionStatement (fc)); + stmt.ccodenode = cn; + } - cn.append (stmt.body.ccodenode); + public override void visit_unlock_statement (UnlockStatement stmt) { + var l = get_lock_expression (stmt, stmt.resource); - fc = new CCodeFunctionCall (new CCodeIdentifier (((Method) mutex_type.scope.lookup ("unlock")).get_cname ())); + var fc = new CCodeFunctionCall (new CCodeIdentifier (((Method) mutex_type.scope.lookup ("unlock")).get_cname ())); fc.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, l)); - cn.append (new CCodeExpressionStatement (fc)); + var cn = new CCodeFragment (); + cn.append (new CCodeExpressionStatement (fc)); stmt.ccodenode = cn; } diff --git a/codegen/valaccodegenerator.vala b/codegen/valaccodegenerator.vala index fd465d17c..f8af697d0 100644 --- a/codegen/valaccodegenerator.vala +++ b/codegen/valaccodegenerator.vala @@ -228,6 +228,10 @@ public class Vala.CCodeGenerator : CodeGenerator { head.visit_lock_statement (stmt); } + public override void visit_unlock_statement (UnlockStatement stmt) { + head.visit_unlock_statement (stmt); + } + public override void visit_delete_statement (DeleteStatement stmt) { head.visit_delete_statement (stmt); } diff --git a/codegen/valaccodemodule.vala b/codegen/valaccodemodule.vala index d8e88e316..0355e0c27 100644 --- a/codegen/valaccodemodule.vala +++ b/codegen/valaccodemodule.vala @@ -207,6 +207,10 @@ public abstract class Vala.CCodeModule { next.visit_lock_statement (stmt); } + public virtual void visit_unlock_statement (UnlockStatement stmt) { + next.visit_unlock_statement (stmt); + } + public virtual void visit_delete_statement (DeleteStatement stmt) { next.visit_delete_statement (stmt); } diff --git a/vala/Makefile.am b/vala/Makefile.am index 69f0d0ac6..aa0b861f4 100644 --- a/vala/Makefile.am +++ b/vala/Makefile.am @@ -151,6 +151,7 @@ libvalacore_la_VALASOURCES = \ valatypeparameter.vala \ valatypesymbol.vala \ valaunaryexpression.vala \ + valaunlockstatement.vala \ valaunresolvedsymbol.vala \ valaunresolvedtype.vala \ valausingdirective.vala \ diff --git a/vala/valacodevisitor.vala b/vala/valacodevisitor.vala index f11a3ef43..55f87d7cc 100644 --- a/vala/valacodevisitor.vala +++ b/vala/valacodevisitor.vala @@ -404,6 +404,14 @@ public abstract class Vala.CodeVisitor { public virtual void visit_lock_statement (LockStatement stmt) { } + /** + * Visit operation called for unlock statements. + * + * @param stmt an unlock statement + */ + public virtual void visit_unlock_statement (UnlockStatement stmt) { + } + /** * Visit operation called for delete statements. * diff --git a/vala/valaflowanalyzer.vala b/vala/valaflowanalyzer.vala index ff5f255db..193f6103a 100644 --- a/vala/valaflowanalyzer.vala +++ b/vala/valaflowanalyzer.vala @@ -1010,8 +1010,12 @@ public class Vala.FlowAnalyzer : CodeVisitor { if (unreachable (stmt)) { return; } + } - stmt.body.accept (this); + public override void visit_unlock_statement (UnlockStatement stmt) { + if (unreachable (stmt)) { + return; + } } public override void visit_expression (Expression expr) { diff --git a/vala/valalockstatement.vala b/vala/valalockstatement.vala index 9ad5dfbdd..da9a0be94 100644 --- a/vala/valalockstatement.vala +++ b/vala/valalockstatement.vala @@ -1,5 +1,6 @@ /* valalockstatement.vala * + * Copyright (C) 2009 Jiří Zárevúcky * Copyright (C) 2006-2007 Raffaele Sandrini, Jürg Billeter * * This library is free software; you can redistribute it and/or @@ -16,14 +17,19 @@ * License along with this library; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA * - * Author: + * Authors: * Raffaele Sandrini + * Jiří Zárevúcky */ using GLib; /** - * Represents a lock statement e.g. {{{ lock (a) { f(a) } }}}. + * Represents a lock statement e.g. {{{ lock (a); }}} or {{{ lock (a) { f(a); } }}}. + * + * If the statement is empty, the mutex remains locked until a corresponding UnlockStatement + * occurs. Otherwise it's translated into a try/finally statement which unlocks the mutex + * after the block is finished. */ public class Vala.LockStatement : CodeNode, Statement { /** @@ -34,9 +40,9 @@ public class Vala.LockStatement : CodeNode, Statement { /** * The statement during its execution the resource is locked. */ - public Block body { get; set; } + public Block? body { get; set; } - public LockStatement (Expression resource, Block body, SourceReference? source_reference = null) { + public LockStatement (Expression resource, Block? body, SourceReference? source_reference = null) { this.body = body; this.source_reference = source_reference; this.resource = resource; @@ -44,11 +50,29 @@ public class Vala.LockStatement : CodeNode, Statement { public override void accept (CodeVisitor visitor) { resource.accept (visitor); - body.accept (visitor); + if (body != null) { + body.accept (visitor); + } visitor.visit_lock_statement (this); } public override bool check (SemanticAnalyzer analyzer) { + if (body != null) { + // if the statement isn't empty, it is converted into a try statement + + var fin_body = new Block (source_reference); + fin_body.add_statement (new UnlockStatement (resource, source_reference)); + + var block = new Block (source_reference); + block.add_statement (new LockStatement (resource, null, source_reference)); + block.add_statement (new TryStatement (body, fin_body, source_reference)); + + var parent_block = (Block) parent_node; + parent_block.replace_statement (this, block); + + return block.check (analyzer); + } + if (checked) { return !error; } @@ -56,11 +80,10 @@ public class Vala.LockStatement : CodeNode, Statement { checked = true; resource.check (analyzer); - body.check (analyzer); /* resource must be a member access and denote a Lockable */ if (!(resource is MemberAccess && resource.symbol_reference is Lockable)) { - error = true; + error = true; resource.error = true; Report.error (resource.source_reference, "Expression is either not a member access or does not denote a lockable member"); return false; @@ -68,7 +91,7 @@ public class Vala.LockStatement : CodeNode, Statement { /* parent symbol must be the current class */ if (resource.symbol_reference.parent_symbol != analyzer.current_class) { - error = true; + error = true; resource.error = true; Report.error (resource.source_reference, "Only members of the current class are lockable"); } diff --git a/vala/valaunlockstatement.vala b/vala/valaunlockstatement.vala new file mode 100644 index 000000000..e0ae1a034 --- /dev/null +++ b/vala/valaunlockstatement.vala @@ -0,0 +1,70 @@ +/* valaunlockstatement.vala + * + * Copyright (C) 2009 Jiří Zárevúcky, Jürg Billeter + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * Author: + * Jiří Zárevúcky + */ + +/** + * Represents an unlock statement e.g. {{{ unlock (a); }}}. + */ +public class Vala.UnlockStatement : CodeNode, Statement { + /** + * Expression representing the resource to be unlocked. + */ + public Expression resource { get; set; } + + public UnlockStatement (Expression resource, SourceReference? source_reference = null) { + this.source_reference = source_reference; + this.resource = resource; + } + + public override void accept (CodeVisitor visitor) { + resource.accept (visitor); + visitor.visit_unlock_statement (this); + } + + public override bool check (SemanticAnalyzer analyzer) { + if (checked) { + return !error; + } + + checked = true; + + resource.check (analyzer); + + /* resource must be a member access and denote a Lockable */ + if (!(resource is MemberAccess && resource.symbol_reference is Lockable)) { + error = true; + resource.error = true; + Report.error (resource.source_reference, "Expression is either not a member access or does not denote a lockable member"); + return false; + } + + /* parent symbol must be the current class */ + if (resource.symbol_reference.parent_symbol != analyzer.current_class) { + error = true; + resource.error = true; + Report.error (resource.source_reference, "Only members of the current class are lockable"); + } + + ((Lockable) resource.symbol_reference).set_lock_used (true); + + return !error; + } +}