Allegro pull requests

[DarkDragon]

FYI, ZoriaRPG got in touch with the Allegro folks and it sounds like they're willing to process pull requests for bugfixes to the 4.4 branch that we send them. I've already filed one containing our modifications (https://github.com/liballeg/allegro5/pull/789) but if you fix the keyboard bug, etc, consider also sending it upstream; it'd be nice not to have to maintain our own patched version of Allegro. Feel free to use the allegro5 fork in the AGN github account as a staging area.

[ZoriaRPG]

Aye. Edgar Reynaldo over on allegro.cc is being just brilliant on this subject.

I went in expecting to get the old story of 'Just upgrade to Allegro 5', but instead I find that there is better support for ag4 now, than there was in 2011; so it would be prudent for each of us to participate on the forums there, and try to fix as much as possible in ag 4.4.3, and aim for a proper release of that.

Here is a changelog for allegro 4.4.3 over 4.4.2.

Insofar as they KB issue, I am having problems making the bug occur in any ZC build at present, including in 2.50.2. If it is a race condition as @Saffith suspects, then I am unsure how to trigger it. If it is a library issue as @Gleeok and I believe, then it may have been fixed--hard to tell just yet.

It would be great if all of our special needs, and out fixes, are simply in the core library though.

I have a few live topics over on allegro.cc if anyone wishes to participate in them.

[ZoriaRPG]

Aye. Edgar Reynaldo over on allegro.cc is being just brilliant on this subject.

I went in expecting to get the old story of 'Just upgrade to Allegro 5', but instead I find that there is better support for ag4 now, than there was in 2011; so it would be prudent for each of us to participate on the forums there, and try to fix as much as possible in ag 4.4.3, and aim for a proper release of that.

Here is a [url=https://pastebin.com/JinsWu26]changelog for allegro 4.4.3 ]/url]over 4.4.2.

Insofar as they KB issue, I am having problems making the bug occur in any ZC build at present, including in 2.50.2. If it is a race condition as @Saffith suspects, then I am unsure how to trigger it. If it is a library issue as @Gleeok and I believe, then it may have been fixed--hard to tell just yet.

It would be great if all of our special needs, and out fixes, are simply in the core library though.

I have a few live topics over on allegro.cc if anyone wishes to participate in them.

[ZoriaRPG]

[DP; remove]

[DarkDragon]

Aye. Edgar Reynaldo over on allegro.cc is being just brilliant on this subject.

I went in expecting to get the old story of 'Just upgrade to Allegro 5', but instead I find that there is better support for ag4 now, than there was in 2011; so it would be prudent for each of us to participate on the forums there, and try to fix as much as possible in ag 4.4.3, and aim for a proper release of that.

Here is a [url=https://pastebin.com/JinsWu26]changelog for allegro 4.4.3 ]/url]over 4.4.2.

Insofar as they KB issue, I am having problems making the bug occur in any ZC build at present, including in 2.50.2. If it is a race condition as @Saffith suspects, then I am unsure how to trigger it. If it is a library issue as @Gleeok and I believe, then it may have been fixed--hard to tell just yet.

It would be great if all of our special needs, and out fixes, are simply in the core library though.

I have a few live topics over on allegro.cc if anyone wishes to participate in them.

The code snippet that Saffith posted on Pure is definitely buggy:

buffer->lock++;

   if (buffer->lock != 1) {
      buffer->lock--;
      return;
   }

   // Do stuff

   buffer->lock--;

C++ is not required to compile lock++ into an atomic operation, in which case the following sequence is possible:

Thread 1 reads the value of lock (0)
Thread 2 reads the value of locK (0)
Thread 1 post-increments lock (now 1) and enters the critical section
Thread 2 post-increments lock (writing 1 again) and enters the critical section
Both threads decrement lock (now -1).

As with all concurrency bugs, however, even though the code is objectively incorrect, it might happen to work on some compilers on some machines, where the increment happens to be atomic; or the bug might be present but never exercised due to precise timings of context switches on some particular machine.

[ZoriaRPG]

The code snippet that Saffith posted on Pure is definitely buggy:

buffer->lock++;

   if (buffer->lock != 1) {
      buffer->lock--;
      return;
   }

   // Do stuff

   buffer->lock--;

C++ is not required to compile lock++ into an atomic operation, in which case the following sequence is possible:

Thread 1 reads the value of lock (0)
Thread 2 reads the value of locK (0)
Thread 1 post-increments lock (now 1) and enters the if block
Thread 2 post-increments lock (writing 1 again) and enters the if block
Both threads decrement lock (now -1).

As with all concurrency bugs, however, even though the code is objectively incorrect, it might happen to work on some compilers on some machines, where the increment happens to be atomic; or the bug might be present but never exercised due to precise timings of context switches on some particular machine.

Wouldn't a basic statement of :

 if ( buffer->lock > 0 ) buffer->lock--;

help there?

That way, the second thread would not touch it.

[DarkDragon]

Nope. This has the exact same problem as the original code.

[ZoriaRPG]

Nope. This has the exact same problem as the original code.

I'm not really sure how to make this an atomic operation that is respected in each of our various compilers, and on every architecture that we use. I think it would be different between gcc/MinGW, and MSVC, and possibly between different MSVC versions; plus it may be different from Windows to Liux and OSX (FreeBSD).

If any if you are clever-enough to know a solution, I'm all eyes. Is using std::atomic for the variable sufficient for our needs, or will that fail based on the compiler? I'm not sure at what point this was introduced as art of the cpp spec, to keep this cpp08 happy.

Which allegro file is this, BTW, so that I can post the problem on allegro.cc? ?

[DarkDragon]

Right. It's one of the many things that is a lot easier to do in C++11 than C++03. I'm pretty sure Allegro already has a mutex implementation somewhere, though.

[ZoriaRPG]

Right. It's one of the many things that is a lot easier to do in C++11 than C++03. I'm pretty sure Allegro already has a mutex implementation somewhere, though.

I could still use a file ref where this code lives. For some reason, my search function started to refuse to give any results on any source code, which if quite, quite, fun.

P.S. I would search the GH branch, but IIRC, GH does not permit searching branches.

Searching the build for ALL_ZC in MSVC returns nothing.

Never mind. The file is keyboard.c as a deep search with DOpus discovered.