Re: [Firebird-devel] Semantics of release
On 05/16/11 21:00, Vlad Khorsun wrote: We can introduce some special mode to report such errors using exceptions, or write message into log, or even provide callback for debugging purposes. But Adriano's question was about *semantics*. So, do you agree with me that explicit detach() must be called by good code and not just release() ? And that call of release() when refCount == 1 is a bug in caller's code ? Regards, Vlad -- Achieve unprecedented app performance and reliability What every C/C++ and Fortran developer should know. Learn how Intel has extended the reach of its next-generation tools to help boost performance applications - inlcuding clusters. http://p.sf.net/sfu/intel-dev2devmay Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel -- What Every C/C++ and Fortran developer Should Know! Read this article and learn how Intel has extended the reach of its next-generation tools to help Windows* and Linux* C/C++ and Fortran developers boost performance applications - including clusters. http://p.sf.net/sfu/intel-dev2devmay Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
Re: [Firebird-devel] Semantics of release
On 05/17/11 19:34, Adriano dos Santos Fernandes wrote:
On 17/05/2011 04:05, Alex Peshkoff wrote:
In FB2.5 yValve did not need any more MT-safeness except provided by
atomic counters and some helper locks like hanlers' map RwLock.
Initially I've planned to keep same approach for FB3. But I did not
review latest Adriano's changes from this POV.
I already show you that 2.5 yvalve is not thread safe. Once more.
How do you expect this code to work concurrently?
--
Statement statement = translateCStatement(stmt_handle);
statement-checkPrepared();
sqlda_sup::dasup_clause clause =
statement-das.dasup_clauses[DASUP_CLAUSE_select];
if (clause.dasup_info_len clause.dasup_info_buf)
{
iterative_sql_info(status,
stmt_handle,
sizeof(describe_select_info),
describe_select_info,
clause.dasup_info_len,
clause.dasup_info_buf,
dialect,
sqlda);
}
--
Accessing same memory, extending internal buffers, etc.
Good point, but I did not mean abuse of API. I only wanted to say that
under normal circumstances there are no race conditions that may cause
server to segfault/hang.
--
What Every C/C++ and Fortran developer Should Know!
Read this article and learn how Intel has extended the reach of its
next-generation tools to help Windows* and Linux* C/C++ and Fortran
developers boost performance applications - including clusters.
http://p.sf.net/sfu/intel-dev2devmay
Firebird-Devel mailing list, web interface at
https://lists.sourceforge.net/lists/listinfo/firebird-devel
Re: [Firebird-devel] Semantics of release
On 17/05/2011 08:27, Vlad Khorsun wrote: What about the following - we always pass status vector to release(). I'm sorry but i against it. addRef\release is and well known pair and its usage also well known. I see no good reason to change it. More, if we need to change it, then we use it in a wrong way. ... It's not a problem technically - IStatus is not refCounted. We use it in Call detach() is also not a problem technically, why don't use it when necessary ? Why we want to replace detach by release ??? ... the following way - if refCounter 1, no error happened (later it always means that nothing like system call failed took place). If refCounter == 1 and object has no special dtor, again no error happened. If object have no explicit destructor then release must be called, no problem. Your words are very incongruent. Release releases, and shall be valid for any objects. You broke the IAttachment from external engines, with are only release-able. They should not be dropped or detached. Making the release an error is like create private destructors. You could do it, but difficultly you may justify it. But if we release something like transaction and due to it rollback some job - transaction is released (what can we do?), but error (or may be better warning? not sure...) is reported - job was implicitly rolled back. You can kill me, but i don't understand - why do we want to call release, if good special explicit destructor is present ??? ... Why are we adding addRef/release after all? To support wrong code and solve nothing. With the cost of all this mess, all this otherwise unneeded discussion, all the global/local pool mess. Adriano -- Achieve unprecedented app performance and reliability What every C/C++ and Fortran developer should know. Learn how Intel has extended the reach of its next-generation tools to help boost performance applications - inlcuding clusters. http://p.sf.net/sfu/intel-dev2devmay Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
Re: [Firebird-devel] Semantics of release
Speaking about documentation i'd said we must document usage of every public interface and specify explicitly how instance is constructed and destroyed. Well - with this adjustment I can agree with every style. (Though must say that addRef/release pair in our interfaces is already far-far away from how it's used in things like OLE2. Do you want to rename them?) PS You make me remember MS way to report errors in their DAC's. They have global Errors collection and it must be checked after every call of any object. Or remember GetLastError. Do you want something similar ? We can remove IStatus parameter from every method and add IStatus* IMaster::getStatus() to the master interface. Not sure i like it... Definitely dislike. This looks VERY bad from MT POV. What if one wants to check for errors in the thread distinct from that in which function was called? (I know it looks strange, but we really had such a case with SAS). That's all are errno-like variations, and people used to have problems with errno in MT world, and why should we once again repeat all that steps? -- Achieve unprecedented app performance and reliability What every C/C++ and Fortran developer should know. Learn how Intel has extended the reach of its next-generation tools to help boost performance applications - inlcuding clusters. http://p.sf.net/sfu/intel-dev2devmay Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
Re: [Firebird-devel] Semantics of release
On 05/17/11 16:07, Vlad Khorsun wrote: To support wrong code and solve nothing. With the cost of all this mess, all this otherwise unneeded discussion, all the global/local pool mess. ...only emotions and offence... Agreed - nothing except emotions ... -- Achieve unprecedented app performance and reliability What every C/C++ and Fortran developer should know. Learn how Intel has extended the reach of its next-generation tools to help boost performance applications - inlcuding clusters. http://p.sf.net/sfu/intel-dev2devmay Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
Re: [Firebird-devel] Semantics of release
On 17/05/2011 04:05, Alex Peshkoff wrote:
In FB2.5 yValve did not need any more MT-safeness except provided by
atomic counters and some helper locks like hanlers' map RwLock.
Initially I've planned to keep same approach for FB3. But I did not
review latest Adriano's changes from this POV.
I already show you that 2.5 yvalve is not thread safe. Once more.
How do you expect this code to work concurrently?
--
Statement statement = translateCStatement(stmt_handle);
statement-checkPrepared();
sqlda_sup::dasup_clause clause =
statement-das.dasup_clauses[DASUP_CLAUSE_select];
if (clause.dasup_info_len clause.dasup_info_buf)
{
iterative_sql_info(status,
stmt_handle,
sizeof(describe_select_info),
describe_select_info,
clause.dasup_info_len,
clause.dasup_info_buf,
dialect,
sqlda);
}
--
Accessing same memory, extending internal buffers, etc.
Adriano
PS: There is a paper
(http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf) from Scott
Meyers and Andrei Alexandrescu showing even or volatile usage in base
classes are wrong. Unfortunately it's down.
--
Achieve unprecedented app performance and reliability
What every C/C++ and Fortran developer should know.
Learn how Intel has extended the reach of its next-generation tools
to help boost performance applications - inlcuding clusters.
http://p.sf.net/sfu/intel-dev2devmay
Firebird-Devel mailing list, web interface at
https://lists.sourceforge.net/lists/listinfo/firebird-devel
Re: [Firebird-devel] Semantics of release
PS: There is a paper (http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf) from Scott Meyers and Andrei Alexandrescu showing even or volatile usage in base classes are wrong. Unfortunately it's down. With MSVC we are safe as we use volatile and compiler used read\write memory barriers (or acquire\release semantics) for access to volatile variables : http://msdn.microsoft.com/en-us/library/ms686355.aspx Can't said for other compilers (GCC at the first place). Regards, Vlad -- What Every C/C++ and Fortran developer Should Know! Read this article and learn how Intel has extended the reach of its next-generation tools to help Windows* and Linux* C/C++ and Fortran developers boost performance applications - including clusters. http://p.sf.net/sfu/intel-dev2devmay Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
Re: [Firebird-devel] Semantics of release
On 17-05-2011 19:58, Vlad Khorsun wrote: PS: There is a paper (http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf) from Scott Meyers and Andrei Alexandrescu showing even or volatile usage in base classes are wrong. Unfortunately it's down. With MSVC we are safe as we use volatile and compiler used read\write memory barriers (or acquire\release semantics) for access to volatile variables : http://msdn.microsoft.com/en-us/library/ms686355.aspx Can't said for other compilers (GCC at the first place). Most usage of volatile to implement double-check locking pattern is in platform neutral code, hence wrong. Adriano -- What Every C/C++ and Fortran developer Should Know! Read this article and learn how Intel has extended the reach of its next-generation tools to help Windows* and Linux* C/C++ and Fortran developers boost performance applications - including clusters. http://p.sf.net/sfu/intel-dev2devmay Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
Re: [Firebird-devel] Semantics of release
On 17-05-2011 19:58, Vlad Khorsun wrote: PS: There is a paper (http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf) from Scott Meyers and Andrei Alexandrescu showing even or volatile usage in base classes are wrong. Unfortunately it's down. With MSVC we are safe as we use volatile and compiler used read\write memory barriers (or acquire\release semantics) for access to volatile variables : http://msdn.microsoft.com/en-us/library/ms686355.aspx Can't said for other compilers (GCC at the first place). Most usage of volatile to implement double-check locking pattern is in platform neutral code, hence wrong. We can or use compiler-\arch- specific read\write barriers, or use interlocked access for InitMutex::flag and InitInstance::flag. At first look i'm not found very hot singletons at our code, so cost of full memory barriers (because of interlocked access) could be acceptable for us. Regards, Vlad -- What Every C/C++ and Fortran developer Should Know! Read this article and learn how Intel has extended the reach of its next-generation tools to help Windows* and Linux* C/C++ and Fortran developers boost performance applications - including clusters. http://p.sf.net/sfu/intel-dev2devmay Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
Re: [Firebird-devel] Semantics of release
On 05/16/11 20:33, Vlad Khorsun wrote: Could you explain, please, semantics of addRef/release on public provider interfaces? The issue was discussed on this list with two possibilities: - when refCount = 1, release does things like detach, rollback, free, etc. It's what was implemented so far. - sometime mentioned also that detach/rollback/etc would not release, and user would need to explicit call it. But now Vlad says we can't call release when refCount = 1, and it's just there to decrement addRefs. This broke code I wrote based on discussed ideas, and I don't feel it's correct. (Also note Alex code in jrd.cpp and is clear he wrote code not using this new assumption.) I assume there must be a kind of symmetry : attach\detach, startTx\commit, startTx\rollback, allocateStmt\freeStmt, addRef\release. From semantic POV attach\detach is like constructor\destructor pair while addRef\release should be used for additional references only (i.e. not instead of destructor). So, if code calls attach\release - this is bug, as for me, and i see no reasons to write code in this way intentionally. I consider at as very bad practice as it could lead to the inexpected side effects (such as implicit rollback). Unfortunately release() is also a kind of dtor, and we can't report about an error that happened inside release(). Therefore the question is - what to do if one called release() for an object with refCount == 1? I assume that for us this is a kind of broken network connection, i.e. attachment is closed, transactions rolled back, etc. Bad style - but we must do something here... Certainly, nothing prevents us from adding error reporting to our release. Should we do it? -- Achieve unprecedented app performance and reliability What every C/C++ and Fortran developer should know. Learn how Intel has extended the reach of its next-generation tools to help boost performance applications - inlcuding clusters. http://p.sf.net/sfu/intel-dev2devmay Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
Re: [Firebird-devel] Semantics of release
On 05/16/11 20:33, Vlad Khorsun wrote: Could you explain, please, semantics of addRef/release on public provider interfaces? The issue was discussed on this list with two possibilities: - when refCount = 1, release does things like detach, rollback, free, etc. It's what was implemented so far. - sometime mentioned also that detach/rollback/etc would not release, and user would need to explicit call it. But now Vlad says we can't call release when refCount = 1, and it's just there to decrement addRefs. This broke code I wrote based on discussed ideas, and I don't feel it's correct. (Also note Alex code in jrd.cpp and is clear he wrote code not using this new assumption.) I assume there must be a kind of symmetry : attach\detach, startTx\commit, startTx\rollback, allocateStmt\freeStmt, addRef\release. From semantic POV attach\detach is like constructor\destructor pair while addRef\release should be used for additional references only (i.e. not instead of destructor). So, if code calls attach\release - this is bug, as for me, and i see no reasons to write code in this way intentionally. I consider at as very bad practice as it could lead to the inexpected side effects (such as implicit rollback). Unfortunately release() is also a kind of dtor, and we can't report about an error that happened inside release(). Therefore the question is - what to do if one called release() for an object with refCount == 1? I assume that for us this is a kind of broken network connection, i.e. attachment is closed, transactions rolled back, etc. Bad style - but we must do something here... Sure, we must do something and current code seems more-or-less OK in this regards. Certainly, nothing prevents us from adding error reporting to our release. Should we do it? We can introduce some special mode to report such errors using exceptions, or write message into log, or even provide callback for debugging purposes. But Adriano's question was about *semantics*. So, do you agree with me that explicit detach() must be called by good code and not just release() ? And that call of release() when refCount == 1 is a bug in caller's code ? Regards, Vlad -- Achieve unprecedented app performance and reliability What every C/C++ and Fortran developer should know. Learn how Intel has extended the reach of its next-generation tools to help boost performance applications - inlcuding clusters. http://p.sf.net/sfu/intel-dev2devmay Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
Re: [Firebird-devel] Semantics of release
On 16/05/2011 13:33, Vlad Khorsun wrote: So, if code calls attach\release - this is bug, as for me, and i see no reasons to write code in this way intentionally. I consider at as very bad practice as it could lead to the inexpected side effects (such as implicit rollback). Now every provider class has a way to release with another methods, Again, release it not how object should be finally destroyed. It is for additional references *only*. but take into account that release is base method. Not sure i understand what do you want to say here. addRef\release used for reference counting and not for creating\destructing objects. But what about different release-able classes? Are you going to add additional methods to release them, cause release is only for addRef? Again not sure i understand you correctly. Basically, if something have *explicit* constructor (attach) it must have explicit destructor (detach). If something is not constructed *explicitly* (fb_get_master_interface) it should be just release()'ed without additional methods. Yes, in a very simple cases we can omit creation of explicit destructor but i prefer to have it even in simplest case. Your never know if tomorrow simplest thing will not became more complex ;) Show me example of what do you mean by different release-able classes and i'll try to be more precisely. Regards, Vlad -- Achieve unprecedented app performance and reliability What every C/C++ and Fortran developer should know. Learn how Intel has extended the reach of its next-generation tools to help boost performance applications - inlcuding clusters. http://p.sf.net/sfu/intel-dev2devmay Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
Re: [Firebird-devel] Semantics of release
On 16/05/2011 14:12, Vlad Khorsun wrote: On 16/05/2011 13:33, Vlad Khorsun wrote: So, if code calls attach\release - this is bug, as for me, and i see no reasons to write code in this way intentionally. I consider at as very bad practice as it could lead to the inexpected side effects (such as implicit rollback). Now every provider class has a way to release with another methods, Again, release it not how object should be finally destroyed. It is for additional references *only*. This is something new you invented. but take into account that release is base method. Not sure i understand what do you want to say here. addRef\release used for reference counting and not for creating\destructing objects. See above. But what about different release-able classes? Are you going to add additional methods to release them, cause release is only for addRef? Again not sure i understand you correctly. Basically, if something have *explicit* constructor (attach) it must have explicit destructor (detach). If something is not constructed *explicitly* (fb_get_master_interface) it should be just release()'ed without additional methods. Everything is constructed explicitly. Yes, in a very simple cases we can omit creation of explicit destructor but i prefer to have it even in simplest case. Your never know if tomorrow simplest thing will not became more complex ;) Show me example of what do you mean by different release-able classes and i'll try to be more precisely. IFirebirdConf and IPluginConf, for example. They're released with release. The situation with new classes trying to be smart using reference counting is still almost unexplained from the POV of why that was done. Sure you may create artificial and specific examples of where they make things to not crash. But nothing except an atomic counter is currently being cared regard MT-safeness. Y-valve has internal state not synchronized, for example. Y-valve delegates call to another providers, and they do nothing re. died objects to return information. Adriano -- Achieve unprecedented app performance and reliability What every C/C++ and Fortran developer should know. Learn how Intel has extended the reach of its next-generation tools to help boost performance applications - inlcuding clusters. http://p.sf.net/sfu/intel-dev2devmay Firebird-Devel mailing list, web interface at https://lists.sourceforge.net/lists/listinfo/firebird-devel
