Menu
▾
▴
firebird-devel
|
From: Alex P. <pes...@ma...> - 2011-03-31 14:17:21
|
On 03/31/11 17:52, Kjell Rilbe wrote: > Adriano dos Santos Fernandes skriver: >> QueryInterface uses GUIDs, and an interface with a GUID should not >> change. >> >> This is not conceptually compatible with our versioning scheme. > > If I, as a mere deadly FB user, may butt in here, I really really > think QueryInterface and IUnkown compatibility is not good for FB. > > 1. FB is multi platform, while IUnknown is inherently tied to > Microsoft. (Or am I wrong about this?) > > 2. IUnknown compatibility would imply a lot of other stuff that FB > doesn't want, re. COM etc. Although that may not be required per se, > just because of IUnknown compatibility, it is in people's minds very > tightly coupled with COM. This is not good. FB is not a COM object and > shouldn't be. Ever. Please. ;-) > > I think QueryInterface and IUnkown would confuse more than help. Please do not think that I suggest to add it. :-) I just want to say that this is possible without major changes. |
|
From: Alex P. <pes...@ma...> - 2011-03-31 14:16:40
|
On 03/31/11 17:45, Adriano dos Santos Fernandes wrote: > On 31-03-2011 10:30, Alex Peshkoff wrote: >> On 03/31/11 16:21, Vlad Khorsun wrote: >>>> On 03/31/11 15:28, Vlad Khorsun wrote: >>>>>> On the other hand I see no problems with adding that method to our >>>>>> interfaces, specially if it's needed to make Delphi people life easier. >>>>>> It does not conflict with our versioning support. >>>>> Unfortunately it is not enough. To be binary compatible with IUnknown >>>>> (not with Delphi itself but with well known stantard interface) we should use >>>>> stdcall also. So, we can be compatible and abandon upgradeInterface, or >>>>> not compatible and don't add confusion introducing queryInterface. >>>> Vlad, if really needed we can make 3 first functions in our interface to >>>> use stdcall. This breaks nothing. >>> And all other functions still will be cdecl ? Don't looks as beauty to me :( >> Vlad, I can agree that this is a bit strange, but if we forget about >> emotions - absolutely nothing bad. >> Functions from base Firebird::Interface never require upgrade - they are >> always present. > QueryInterface uses GUIDs, and an interface with a GUID should not change. > > This is not conceptually compatible with our versioning scheme. Nobody said that GUID should remain the same when version changes. It's even possible to return pointer to current class on request for old GUID. |
|
From: Adriano d. S. F. <adr...@gm...> - 2011-03-31 15:13:35
|
On 31-03-2011 07:49, Alex Peshkoff wrote: >> People would like to detach a parent and not care about statements >> created but unfreed for this attachment. >> > > If they do not call addRef for statements, they will be destroyed when > attachment goes away > This is against of what you already said, and in fact destroys the concept of recounting. If I call attachment->allocateStatement, this statement is owned by me now, even if I don't call addRef on it. If someone else calls release on it, they must have called addRef before. Then, you have leaks if one releases the attachment and leave the statement. So again, two different bad things for two different choices. >> And for who is using C++ and RAII, they don't need addRef/release in our >> objects to use that. > > Absolutely wrong. > "Wrong" because you want to support bad practices, of use a object after it's freed. >> Well, if you want to say that someone is correct just because chose the >> COM way, then we would need to say we're incorrect because we don't use >> queryInterface... > > In many aspects use of queryInterface might be ideal. Two main reasons > why we can't use it: > - it will cause additional delays in time-critical places like fetching > next record, > - it will pollute each place in the code where we work with plugins. > > On the other hand I see no problems with adding that method to our > interfaces, specially if it's needed to make Delphi people life easier. > It does not conflict with our versioning support. > This is looking like a Frankenstein... Adriano |
|
From: Alex P. <pes...@ma...> - 2011-04-01 10:14:51
|
On 03/31/11 19:13, Adriano dos Santos Fernandes wrote: > On 31-03-2011 07:49, Alex Peshkoff wrote: > >>> People would like to detach a parent and not care about statements >>> created but unfreed for this attachment. >>> >> If they do not call addRef for statements, they will be destroyed when >> attachment goes away >> > This is against of what you already said, and in fact destroys the > concept of recounting. > > If I call attachment->allocateStatement, this statement is owned by me > now, even if I don't call addRef on it. > Sorry. Certainly you are right - looks like I was tired in the evening and therefore said such foolish thing. > If someone else calls release on it, they must have called addRef > before. Then, you have leaks if one releases the attachment and leave > the statement. Certainly. If one requested an object, got it and did not release() in any way - there is memory leak. > So again, two different bad things for two different choices. > >>> And for who is using C++ and RAII, they don't need addRef/release in our >>> objects to use that. >> Absolutely wrong. >> > "Wrong" because you want to support bad practices, of use a object after > it's freed. > Looks like what you call 'bad practices' is what almost everyone here wants to support :) Getting serious - certainly, an object can't be used after it's freed. The question is what we call 'freed'. In old API freed meant detach/commit was done, but even after it (if noone allocates about 2**32 handles) correct diagnostics about that usage was provided. That said - certainly people can't use object after it was detached/dropped/etc. But they have reasons to get correct diagnostics instead segfault in such a case. As we have agreed, sometimes it's very hard to guarantee that an attachment is alive even in the next line after attach was called. And what - segfault? >>> Well, if you want to say that someone is correct just because chose the >>> COM way, then we would need to say we're incorrect because we don't use >>> queryInterface... >> In many aspects use of queryInterface might be ideal. Two main reasons >> why we can't use it: >> - it will cause additional delays in time-critical places like fetching >> next record, >> - it will pollute each place in the code where we work with plugins. >> >> On the other hand I see no problems with adding that method to our >> interfaces, specially if it's needed to make Delphi people life easier. >> It does not conflict with our versioning support. >> > This is looking like a Frankenstein... To be precise - I see no tech problem. Logically it's not needed for our interface. |
|
From: Adriano d. S. F. <adr...@gm...> - 2011-04-01 21:34:30
|
On 01-04-2011 07:14, Alex Peshkoff wrote: > On 03/31/11 19:13, Adriano dos Santos Fernandes wrote: >> On 31-03-2011 07:49, Alex Peshkoff wrote: >> >>>> People would like to detach a parent and not care about statements >>>> created but unfreed for this attachment. >>>> >>> If they do not call addRef for statements, they will be destroyed when >>> attachment goes away >>> >> This is against of what you already said, and in fact destroys the >> concept of recounting. >> >> If I call attachment->allocateStatement, this statement is owned by me >> now, even if I don't call addRef on it. >> > > Sorry. Certainly you are right - looks like I was tired in the evening > and therefore said such foolish thing. > Now look at this. This, while a not very good practice, is something very used and is how our ISC API work. User detachs an attachment and there is no memory leak from transactions, blobs, statements, etc. And who generally closes all handles? Most probably languages with RAII support, especially C++. But wait, to make RAII work in C++ with our API we don't need refcounters. You're refcounters are to support another bad practice: use after freed. So while them "fix" a very-bad practice, it makes a not so bad to leak memory. >> If someone else calls release on it, they must have called addRef >> before. Then, you have leaks if one releases the attachment and leave >> the statement. > > Certainly. If one requested an object, got it and did not release() in > any way - there is memory leak. > >> So again, two different bad things for two different choices. >> >>>> And for who is using C++ and RAII, they don't need addRef/release in our >>>> objects to use that. >>> Absolutely wrong. >>> >> "Wrong" because you want to support bad practices, of use a object after >> it's freed. >> > > Looks like what you call 'bad practices' is what almost everyone here > wants to support :) > One cannot use free then use a pointer. One cannot delete then use a pointer. This is bad practice. On the other hand, people are used to quit an application and see no memory leaks. They destroy a pool to not delete individual objects. And so on. I think you must really think, your fix to a bad practice is done a damage to normal usage. > Getting serious - certainly, an object can't be used after it's freed. > The question is what we call 'freed'. > In old API freed meant detach/commit was done, but even after it (if > noone allocates about 2**32 handles) correct diagnostics about that > usage was provided. > > That said - certainly people can't use object after it was > detached/dropped/etc. But they have reasons to get correct diagnostics > instead segfault in such a case. As we have agreed, sometimes it's very > hard to guarantee that an attachment is alive even in the next line > after attach was called. And what - segfault? > I will again must say to take another look at the problem. OS kernels has handles or numeric file descriptors. When user mode calls the kernel, kernel verifies it's valid. So as our handles. But if a OS library returns a pointer, there is no protection to wrong usage on them. User must use memory correct, or must use a language with they don't need to deal with it directly. So as our objects must be. Adriano |
|
From: Alex P. <pes...@ma...> - 2011-04-04 12:45:51
|
On 04/02/11 01:34, Adriano dos Santos Fernandes wrote: > On 01-04-2011 07:14, Alex Peshkoff wrote: >> On 03/31/11 19:13, Adriano dos Santos Fernandes wrote: >>> On 31-03-2011 07:49, Alex Peshkoff wrote: >>> >>>>> People would like to detach a parent and not care about statements >>>>> created but unfreed for this attachment. >>>>> >>>> If they do not call addRef for statements, they will be destroyed when >>>> attachment goes away >>>> >>> This is against of what you already said, and in fact destroys the >>> concept of recounting. >>> >>> If I call attachment->allocateStatement, this statement is owned by me >>> now, even if I don't call addRef on it. >>> >> Sorry. Certainly you are right - looks like I was tired in the evening >> and therefore said such foolish thing. >> > Now look at this. This, while a not very good practice, is something > very used and is how our ISC API work. > > User detachs an attachment and there is no memory leak from > transactions, blobs, statements, etc. Not for transactions - one can't detach with active transactions. > And who generally closes all handles? Most probably languages with RAII > support, especially C++. > > But wait, to make RAII work in C++ with our API we don't need > refcounters. You're refcounters are to support another bad practice: use > after freed. Please call it 'help avoid segfault in case of races'. > So while them "fix" a very-bad practice, it makes a not so bad to leak > memory. > I.e. we must choose from 2 bad choices - one may cause memory leaks, other - random segfaults. (Why random: I agree that normally one should not use an object after detach/commit. But unfortunately races exist, and in that case an object, detached in thread A, can be used in thread B.) I prefer to avoid segfaults in case of races. They are very hard to reproduce (for example, buggy code in solaris lock manager existed for a very long time, as long as that code was not tried on linux threads, where it caused segfault due to races). Memory leaks are typically easy reproduced, and there are a lot of tools to deal with memory leaks. One who cares about this can always check for them. On contrary, I do not know a way to avoid races without reference counters built into interfaces except hard locking user code. > One cannot use free then use a pointer. One cannot delete then use a > pointer. This is bad practice. > Certainly bad practice, but once more - just to help in case of races. > On the other hand, people are used to quit an application and see no > memory leaks. They destroy a pool to not delete individual objects. And > so on. > > I think you must really think, your fix to a bad practice is done a > damage to normal usage. > A few lines before you've said yourself that not closing handles is 'a not very good practice'. >> Getting serious - certainly, an object can't be used after it's freed. >> The question is what we call 'freed'. >> In old API freed meant detach/commit was done, but even after it (if >> noone allocates about 2**32 handles) correct diagnostics about that >> usage was provided. >> >> That said - certainly people can't use object after it was >> detached/dropped/etc. But they have reasons to get correct diagnostics >> instead segfault in such a case. As we have agreed, sometimes it's very >> hard to guarantee that an attachment is alive even in the next line >> after attach was called. And what - segfault? >> > I will again must say to take another look at the problem. > > OS kernels has handles or numeric file descriptors. When user mode calls > the kernel, kernel verifies it's valid. So as our handles. Kernel validates everything, including pointers. Some libraries also validate pointers, passed to them, but this is not related to our problem. > But if a OS library returns a pointer, there is no protection to wrong > usage on them. Reference counters were designed to protect pointers, returned by some library. > User must use memory correct, or must use a language with > they don't need to deal with it directly. So as our objects must be. First of all our objects must be oriented to support that languages. And presence of reference counters is the best way to do it. I do not agree when you use as a top priority old languages. I know that they must be _minimally_ supported. But we must have new API oriented on a fresher tools, like C++/Delphi with RAII support. And help people, developing components for tools like .Net implement what they need as easy as possible. For both of this aims reference counters are useful. |
|
From: Adriano d. S. F. <adr...@gm...> - 2011-04-04 12:58:59
|
Alex, Let's stop to waste our time. I'll never agree with your used approach to fix *real user mistakes* adding methods to our API, nor even seems to be enough interested persons to make things better. In the end, the community has what it deserves in the final product, and developers will need to support old decisions. Adriano |
|
From: Dimitry S. <sd...@ib...> - 2011-04-04 13:04:59
|
04.04.2011 14:58, Adriano dos Santos Fernandes wrote: > Let's stop to waste our time. > > I'll never agree with your used approach to fix *real user mistakes* > adding methods to our API, nor even seems to be enough interested > persons to make things better. > > In the end, the community has what it deserves in the final product, and > developers will need to support old decisions. I'd say even more: don't ever expose this interface to public and save whole time which you are going to waste. There is no point to exchange one shit with another. Concentrate your attention on useful things, please. -- SY, SD. |
|
From: Vlad K. <hv...@us...> - 2011-03-30 10:41:43
|
> On 28-03-2011 15:28, Vlad Khorsun wrote:
>>>>> The user should not call detach and another methods simultaneously,
>>>>> never. Why the heck they would do it? This is user application problem.
>>
>> Do you offer us to undo thread-safety changes made in v2.5 client library ? :)
>>
> This has *nothing* related, as 2.5 has no public objects, only handles,
> and nothing in 3.0 changes about it.
Seems you never saw such issues. You are happy ;)
> I'm just saying that we should not protect users from using unallocated
> objects at a price of adding confusing and non-needed functions in our API.
I don't agree with "confusing and non-needed functions". It is not technical and
even not an argument.
> User should *not* request a detachment and then request actions on the
> objects. This *is* user application problem, no mater it being single or
> multi thread.
If *we* will crash with application it will be *our* pain and loss of *our* reputation.
> You and Alex can still use yours refcounting, and in fact I'm still
> using them on my new why.cpp to fix existing problems.
>
> But don't add it to our API, and we don't need it even if you want to
> use proxy objects in the engine.
>
> It's just a mater of using it internally. Yvalve do refcounting and
> don't call methods on unallocated objects.
How YValve objects will be protected ? See above.
> Engine proxies do whatever they need and don't call methods on
> unallocated objects.
>
> But, please, add addRef/release to public API to prevent user from do
> mistakes at the price of do even more evil is not correct.
What evil ? Wish to create robust API is an evil ? Than i'm devil's advocate ;)
Regards,
Vlad
|
|
From: Adriano d. S. F. <adr...@gm...> - 2011-03-30 15:28:12
|
On 30-03-2011 07:41, Vlad Khorsun wrote: >> On 28-03-2011 15:28, Vlad Khorsun wrote: >>>>>> The user should not call detach and another methods simultaneously, >>>>>> never. Why the heck they would do it? This is user application problem. >>> >>> Do you offer us to undo thread-safety changes made in v2.5 client library ? :) >>> >> This has *nothing* related, as 2.5 has no public objects, only handles, >> and nothing in 3.0 changes about it. > > Seems you never saw such issues. You are happy ;) > >> I'm just saying that we should not protect users from using unallocated >> objects at a price of adding confusing and non-needed functions in our API. > > I don't agree with "confusing and non-needed functions". It is not technical and > even not an argument. > First, you and Alex would need to explicit define what's the semantics of release, which is not even deductible by the current trunk. What would do a release in a live (not detached / not committed / etc) object when refcount > 1? What would do a release in a live (not detached / not committed / etc) object when refcount = 1? What would do a detach/commit/etc in an object when refcount > 1? What would do a detach/commit/etc in an object when refcount = 1? Would parents (say attachment is parent of statement) will do statement->addRef/release or not? Same question as above, but in the contrary order, what about relation of child to parents? >> User should *not* request a detachment and then request actions on the >> objects. This *is* user application problem, no mater it being single or >> multi thread. > > If *we* will crash with application it will be *our* pain and loss of *our* reputation. > As I said, there are others ways to wrong application do this. Adriano |
|
From: Vlad K. <hv...@us...> - 2011-03-30 17:45:36
|
>>> I'm just saying that we should not protect users from using unallocated
>>> objects at a price of adding confusing and non-needed functions in our API.
>>
>> I don't agree with "confusing and non-needed functions". It is not technical and
>> even not an argument.
>>
> First, you and Alex would need to explicit define what's the semantics
> of release, which is not even deductible by the current trunk.
Current trunk have known errors in this regards and can't be used as example.
Rule is simple - release() should decrement reference counter. When counter became
zero, object is deleted phisycally. Physical delete should call logical delete if object is in
active state (disconnect, rollback, etc).
> What would do a release in a live (not detached / not committed / etc)
> object when refcount > 1?
Decrement refcount
> What would do a release in a live (not detached / not committed / etc)
> object when refcount = 1?
Decrement refcount and delete object. Destructor (or separate function such as destroy())
will call detach\rollback on real object, if needed.
> What would do a detach/commit/etc in an object when refcount > 1?
Call corresponding method of real object. And not touch refcount.
> What would do a detach/commit/etc in an object when refcount = 1?
Same as above.
> Would parents (say attachment is parent of statement) will do
> statement->addRef/release or not?
Yes. For example, attachment could have array of smartpointers with statements and
in desroy() it will assign NULL's into every item.
> Same question as above, but in the contrary order, what about relation
> of child to parents?
Child's destroy() will ask parent to unregister itself on destroy. If parent already destroying
it will not found such child and it will be ok.
All above is well known and easy to implement techniques.
>>> User should *not* request a detachment and then request actions on the
>>> objects. This *is* user application problem, no mater it being single or
>>> multi thread.
>>
>> If *we* will crash with application it will be *our* pain and loss of *our* reputation.
>>
> As I said, there are others ways to wrong application do this.
We should separate our pointers and user-defined pointers (strings, etc). Else i can
complain that program don't work after i filled random memory regions with garbage ;)
Regards,
Vlad
|
|
From: Dmitry Y. <fir...@ya...> - 2011-04-07 10:55:29
|
30.03.2011 21:45, Vlad Khorsun wrote: >> What would do a detach/commit/etc in an object when refcount> 1? > > Call corresponding method of real object. And not touch refcount. > >> What would do a detach/commit/etc in an object when refcount = 1? > > Same as above. Sorry for ignorance, but why refcount is not decremented in these cases? I mean, what is the point in requiring an extra release() call if the user has explicitly stated that he no longer needs an object. Note that we don't require an explicit addRef() call when the object gets created. Dmitry |
|
From: Vlad K. <hv...@us...> - 2011-04-07 12:09:32
|
> 30.03.2011 21:45, Vlad Khorsun wrote:
>
>>> What would do a detach/commit/etc in an object when refcount> 1?
>>
>> Call corresponding method of real object. And not touch refcount.
>>
>>> What would do a detach/commit/etc in an object when refcount = 1?
>>
>> Same as above.
>
> Sorry for ignorance, but why refcount is not decremented in these cases?
To protect us from the more then one call of detach() for the same instance.
Example :
IAttachment *att = provider->attach(...);
doSomething(att);
att->detach();
doSomething(IAttachment *att)
{
if (something very bad happens) // this could be really at the few levels
att->detach(); // deeper at whole call stack
}
Well, we can decrement refcount in detach() on success only, but it sounds
as (not necessary) complication of reference counting policy.
> I mean, what is the point in requiring an extra release() call if the
> user has explicitly stated that he no longer needs an object. Note that
> we don't require an explicit addRef() call when the object gets created.
I understand your concerns. BTW, changing way to create objects from
IAttachment *attach(...)
by
void attach(..., **IAttachment)
allows us to use smart pointers and make no additional call of release() :
{
smart_ptr<IAttachment> att = NULL;
provider->attach(..., att.refPtr()); // refcnt == 1 here !
doSomething(att);
att->detach(); // refcnt == 1
} // ~smart_ptr will decrement refcnt
Regards,
Vlad
|
|
From: Dmitry Y. <fir...@ya...> - 2011-04-07 12:52:07
|
07.04.2011 16:09, Vlad Khorsun wrote:
> To protect us from the more then one call of detach() for the same instance.
> Example :
>
> IAttachment *att = provider->attach(...);
> doSomething(att);
> att->detach();
>
> doSomething(IAttachment *att)
> {
> if (something very bad happens) // this could be really at the few levels
> att->detach(); // deeper at whole call stack
> }
>
> Well, we can decrement refcount in detach() on success only, but it sounds
> as (not necessary) complication of reference counting policy.
Point taken, thanks.
> I understand your concerns. BTW, changing way to create objects from
>
> IAttachment *attach(...)
>
> by
>
> void attach(..., **IAttachment)
>
> allows us to use smart pointers and make no additional call of release() :
>
> {
> smart_ptr<IAttachment> att = NULL;
> provider->attach(..., att.refPtr()); // refcnt == 1 here !
> doSomething(att);
> att->detach(); // refcnt == 1
> } // ~smart_ptr will decrement refcnt
True, but my concerns are about the languages without smart pointers :-)
Dmitry
|
|
From: Alex P. <pes...@ma...> - 2011-04-07 12:47:17
|
On 04/07/11 14:55, Dmitry Yemanov wrote: > 30.03.2011 21:45, Vlad Khorsun wrote: > >>> What would do a detach/commit/etc in an object when refcount> 1? >> Call corresponding method of real object. And not touch refcount. >> >>> What would do a detach/commit/etc in an object when refcount = 1? >> Same as above. > Sorry for ignorance, but why refcount is not decremented in these cases? > I mean, what is the point in requiring an extra release() call if the > user has explicitly stated that he no longer needs an object. Note that > we don't require an explicit addRef() call when the object gets created. To give good answer to this question we should decide, how exact are we going to follow reference counter logic in OLE2. Formal approach says - number of addRef()/release() calls for an object should match. Constructor creates an object with 0 reference counter. When an object is returned from _any_ function, addRef should be called. This addRef is for the process of returning object - imagine that we return not newly created object but already existing. That object may be used in some other place. If it's reference counter is decreased in some other place to become 0 without such addRef() we may return from function pointer to deallocated memory instead good object. Calling the paired release() is responsibility of code which invoked the function which returned an object. There are 2 typical usages: without smart pointers and with them. In the first case typical code looks like: Object* obj = getObject(...); obj->blaBlaBla(); obj->release(); // refCounter becomes 0, object destroyed With smart pointers: Smart<Object> obj = getObject(...); // refCounter == 2 obj->release(); // refCounter == 1 obj->blaBlaBla(); // In ~Smart() refCounter will become 0 and object will be destroyed If we want detach to decrement refCounter (I must say we can do it if we want, we are not OLE2) first sample looks very good for us: IAttachment* att = provider->attachDatabase(...); att->doSomething(); att->detach(); But with smart pointers we must do the following: Smart<IAttachment> at = ... att->release(); // refCounter == 1 att->blaBlaBla(); We want to detach - and here problems come. We can't simply call: att->detach(); because ~Smart() will try to release an already destroyed object, causing AV. From formal POV we must: att->addRef(); att->detach(); which is correct, but ugly. Or we may do the following: Smart<IAttachment> at = ...; // refCounter == 2 att->blaBlaBla(); att->detach(); // refCounter == 1 // In ~Smart() refCounter will become 0 and object will be destroyed This looks good, but somehow breaks smart pointers - if detach is not done, object will leak. On the other hand, this is like ISC API behaves now. Must say that if we decide to use approach with detach() decrementing refCounter(), people may themself choose, what way do they prefer - from fbclient library POV it will make no difference. |
|
From: Dmitry Y. <fir...@ya...> - 2011-04-07 12:59:34
|
07.04.2011 16:47, Alex Peshkoff wrote: > But with smart pointers we must do the following: > > Smart<IAttachment> at = ... > att->release(); // refCounter == 1 > att->blaBlaBla(); > > We want to detach - and here problems come. We can't simply call: > > att->detach(); > > because ~Smart() will try to release an already destroyed object, > causing AV. True for dumb universal smart pointers. But a smarter one could overload not only operator-> but also the detach() method and mark itself as not requiring release inside the dtor. Dmitry |
|
From: Alex P. <pes...@ma...> - 2011-04-07 13:33:15
|
On 04/07/11 16:59, Dmitry Yemanov wrote: > 07.04.2011 16:47, Alex Peshkoff wrote: > >> But with smart pointers we must do the following: >> >> Smart<IAttachment> at = ... >> att->release(); // refCounter == 1 >> att->blaBlaBla(); >> >> We want to detach - and here problems come. We can't simply call: >> >> att->detach(); >> >> because ~Smart() will try to release an already destroyed object, >> causing AV. > True for dumb universal smart pointers. But a smarter one could overload > not only operator-> but also the detach() method and mark itself as not > requiring release inside the dtor. Certainly. The only thing to decide is how to make people type att.detach(); instead of att->detach(); Possible way is not to have overloaded -> at all, i.e. not expose pointer to the outer world. Instead all required methods should be present in smart class itself. |
|
From: Adriano d. S. F. <adr...@gm...> - 2011-03-28 19:22:16
|
On 28-03-2011 15:28, Vlad Khorsun wrote: >>>> The user should not call detach and another methods simultaneously, >>>> never. Why the heck they would do it? This is user application problem. > > Do you offer us to undo thread-safety changes made in v2.5 client library ? :) > I believe you're too optimistic about thread-safety. No bug reports may not means no problem, it may be because nobody tries to crash "inside our code". Do you really think that isc_dsql_prepare and isc_dsql_free_statement are thread safe when both used concurrent in a same statement handle? This is a question open to finalize my changes. Do we really need to allow simultaneous calls on the same attachment in yvalve, since they're going to be blocked (serialized) in another (remote - not sure, engine - sure) providers? If we say yes, we must do it correct, but current and 2.5 code doesn't look like to be. Adriano |
|
From: Alex P. <pes...@ma...> - 2011-03-29 06:21:37
|
On 03/28/11 23:22, Adriano dos Santos Fernandes wrote: > On 28-03-2011 15:28, Vlad Khorsun wrote: >>>>> The user should not call detach and another methods simultaneously, >>>>> never. Why the heck they would do it? This is user application problem. >> Do you offer us to undo thread-safety changes made in v2.5 client library ? :) >> > I believe you're too optimistic about thread-safety. No bug reports may > not means no problem, it may be because nobody tries to crash "inside > our code". > Must say that I had a number of bug reports from Dmitry Kovalenko, an authot and maintainer of .NET IB provider. He has a very serious test suite for his provider, and certainly when something fails in underlying code he reported that errors. And they were fixed. > Do you really think that isc_dsql_prepare and isc_dsql_free_statement > are thread safe when both used concurrent in a same statement handle? > > This is a question open to finalize my changes. > > Do we really need to allow simultaneous calls on the same attachment in > yvalve, since they're going to be blocked (serialized) in another > (remote - not sure, engine - sure) providers? > Adriano, as far as I remember you were the author of one change in engine code making it possible to have simultaneous calls on the same attachment. It was on SAS request. The fact that calls are serialized now in engine does not mean that we are not planning to (may be) make them run in parallel sometimes in the future. Moreover, detach is already not strictly serialized - it can abort running requests and close database. Therefore I think we must have API ready for simultaneous calls. > If we say yes, we must do it correct, but current and 2.5 code doesn't > look like to be. Agreed. But it does not mean we must stay in that bad state as long as possible. |
|
From: Adriano d. S. F. <adr...@gm...> - 2011-03-29 15:06:54
|
On 29-03-2011 03:21, Alex Peshkoff wrote: > On 03/28/11 23:22, Adriano dos Santos Fernandes wrote: >> On 28-03-2011 15:28, Vlad Khorsun wrote: >>>>>> The user should not call detach and another methods simultaneously, >>>>>> never. Why the heck they would do it? This is user application problem. >>> Do you offer us to undo thread-safety changes made in v2.5 client library ? :) >>> >> I believe you're too optimistic about thread-safety. No bug reports may >> not means no problem, it may be because nobody tries to crash "inside >> our code". >> > > Must say that I had a number of bug reports from Dmitry Kovalenko, an > authot and maintainer of .NET IB provider. He has a very serious test > suite for his provider, and certainly when something fails in underlying > code he reported that errors. And they were fixed. > So, he don't tried what I said, cause the code looks clear to be not thread-safe. >> Do you really think that isc_dsql_prepare and isc_dsql_free_statement >> are thread safe when both used concurrent in a same statement handle? >> >> This is a question open to finalize my changes. >> >> Do we really need to allow simultaneous calls on the same attachment in >> yvalve, since they're going to be blocked (serialized) in another >> (remote - not sure, engine - sure) providers? >> > > Adriano, as far as I remember you were the author of one change in > engine code making it possible to have simultaneous calls on the same > attachment. It was on SAS request. > Where? There were some request for changes, but looks like rolled back due to another problems. The code still lock on the attachment, and it will lock "forever", cause I'm almost sure we'll can't get rid of it *correctly* on the rest of our working life. (it seems in 10 years we didn't fixed simple multi-thread problems) BTW, Oracle does the same, so it's not a absurd. > The fact that calls are serialized now in engine does not mean that we > are not planning to (may be) make them run in parallel sometimes in the > future. Moreover, detach is already not strictly serialized - it can > abort running requests and close database. Therefore I think we must > have API ready for simultaneous calls. > Serialization here is not part of the API. It's implementation details. Currently we try (yes, try) to make a thread-safe code that will serialize later. What's the benefit? If we block now, we can make internal (and working) implementation anytime we want. Adriano |
|
From: Adriano d. S. F. <adr...@gm...> - 2011-03-28 19:26:25
|
On 28-03-2011 04:36, Alex Peshkoff wrote:
> Certainly, more clever helper object can solve a problem. But I do not
> see a way to build it without lock. And I claim that this is too high
> price for removing reference counters from API.
>
I must say here that if you're going to create proxy objects that do:
if (x)
x->something();
And concurrently do:
x = NULL;
let saying, "x" is a Jrd::Attachment or a jrd_tra, this is VERY incorrect.
Adriano
|
|
From: Alex P. <pes...@ma...> - 2011-03-29 06:25:06
|
On 03/28/11 23:26, Adriano dos Santos Fernandes wrote: > On 28-03-2011 04:36, Alex Peshkoff wrote: >> Certainly, more clever helper object can solve a problem. But I do not >> see a way to build it without lock. And I claim that this is too high >> price for removing reference counters from API. >> > I must say here that if you're going to create proxy objects that do: > if (x) > x->something(); > > And concurrently do: > x = NULL; > > let saying, "x" is a Jrd::Attachment or a jrd_tra, this is VERY incorrect. Certainly. But if concurrently we have a call that makes x to become invalid pointer (detach/drop/commit/rollback when used without refCounted objects), this has same effect as x = NULL; |
|
From: Kovalenko D. <dmi...@gm...> - 2011-03-29 10:53:41
|
Hi people I can't read all your messages in this debate But I can say next simple things (forgive me my bad english) 1. At current time, you (FB Developers) do not has any real expirience with object-based interfaces. I right? :) 2. Object-based interfaces should use the refcount. Without this thing, you get the multiple problems. And clients of these interfaces, also. 3. Try think about replace of your Version to normal QueryInterface. If you do not understand now, you understand in future. Five years ago, you did not understand the meaning of benefits of refcount :) I wrote to Alex about the real problems with current "Version" method. 4. Last release of transaction-object with active (but not prepared) transaction should make the rollback this transaction. Without any errors. Last release of connection with active connection should make the detach from database. Without any errors. 5. Addref/Release should be thread-safe (use Interlocked functions). In common case - all methods of all interfaces should be thread-safe. 6. Interface method should return the new object through OUT-parameter like (T** ppObject). In ideal - you should return the pointer to BASE-interface. Do not return (T*) from interface method. This is way for create the mistakes in client code. 7. Do not work with interfaces directly. Use the smart pointers with correct implementation :-) Regards, Kovalenko Dmitry www.ibprovider.com PS. Use doxygen for documentation of your interfaces and prefix "IFB_" for interface names :)))) |
|
From: Dmitry Y. <fir...@ya...> - 2011-04-07 12:44:35
|
All, Speaking honestly, I've got lost in ping ponging between different aspects of the topic. So feel free to correct me if I'll be going a completely wrong way. And please forgive me for writing the obvious things, as I tried to compose a complete picture for myself. The basic requirement we have at hands is to provide full MT safety at the API level. And it was officially stated as a v2.5 feature, so there is no good reason to step back. Ideally, it should have handle destroying to be safe as well. One may claim that this requirement is non-natural and no sane program would be concurrently destroying the handle and working on that handle, but in fact v2.5 works exactly this way -- database and attachment shutdown is asynchronous and may interact with user API calls on the same handle -- and we have it working correctly. So it cannot be completely unexpected for user applications either, be it intentional or not, and a clever error message is surely better than a crash in this case. This requirement is quite easy to achieve with artificial handles like the ones we have prior to v3.0, but any indirect (vtable based) call won't work if the object behind that vtable gets destroyed in the meantime. All the discussion here goes around the fact that the vtable may be invalidated by a concurrent thread. In this case, I don't see any better solution than an explicit reference counting embedded into our API. It's well-known and it solves the problem. And I don't have much to add to the points already raised by Alex and Vlad. However, I see nobody looking from the other side. Why should vtable become broken once the object gets destroyed? It's surely unavoidable if the object is inherited from the returned interface. But does it really have to be so? Imagine a proxy object implementing the returned interface, referencing the dynamic object via an embedded pointer, but allocated out of the global pool and being reused instead of destroyed/recreated. In this case the vtable calls will work always and the actual object can be validated inside like we do now, so the API calls will be returning a proper error (at least until the proxy object gets reused -- but it could be delayed under some rules). I don't say this is a better approach, just something to be mentioned for the sake of completeness. Regarding the other points. As for the y-valve <-> provider interaction, the good (IMNSHO) solution would be the one that does not require the provider to know about its caller. In other words, it may call the y-valve forward to initiate new objects (e.g. to have an external database access) but should not call it back to deal with existing objects (e.g. itself) except calling the hooks passed as parameters. I believe some form of queryInterface/upgradeInterface is required in the API. But I don't think we should mimic IUnknown precisely without really strong reasons. First of all, we don't need GUIDs as interface version identifiers, as our objects are limited to Firebird only usage. And if Delphi is the only reason to be binary compatible with IUnknown, then I'm against it. Times change and now I'd pay more attention to Java/.NET along with Python/PHP/whatever than think how to please those poor Delphi users ;-) Now I'm shutting up and letting you to argue. I've undoubtedly missed something. Dmitry |
|
From: Daniel R. <da...@ac...> - 2011-04-07 13:27:16
|
Hi, At April-07-11, 9:44 AM, Dmitry Yemanov wrote: > I believe some form of queryInterface/upgradeInterface is required in > the API. But I don't think we should mimic IUnknown precisely without > really strong reasons. First of all, we don't need GUIDs as interface > version identifiers, as our objects are limited to Firebird only usage. > And if Delphi is the only reason to be binary compatible with IUnknown, > then I'm against it. Times change and now I'd pay more attention to > Java/.NET along with Python/PHP/whatever than think how to please those > poor Delphi users ;-) Whatever the new style of the Firebird API will be, I'm sure that Delphi will be able to handle it, as long as the data connection developers(IB Objects, FIB Plus, Upscene, Embarcadero, Devart and others) are kept in the loop. And, I've just learned that the new Delphi 64-bit compiler(hopefully released later this year) will only use fastcall for DLL interfaces(even Microsoft has that same restriction for 64-bit). I remember at one point in all of this discussion, there was talks about stdcall and cdecl for the DLL interfaces, but with Delphi 64-bit, it will only support fastcall. And, fastcall is also supported by Delphi 32-bit. -- Best regards, Daniel Rail Senior Software Developer ACCRA Solutions Inc. (www.accra.ca) ACCRA Med Software Inc. (www.filopto.com) |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X