[jitsi-dev] A few issues that I found


#1

Hello all -

I'm fairly new to the Jitsi project so I may lack some background into the issues I'm about to present so please bare with me. My goal is focused primarily around Jitsi's SIP capabilities in producing a (lightweight) java applet that will interface with the browser's UI by way of javascript (LiveConnect). The applet itself will have no UI elements, just a basic API that will be called from a JS library.

I've managed to make it work fairly well as a proof of concept, and I think that speaks volumes to the quality of the Jisti product. So I have to thank all the contributors.

With this background, there are a few issues / bugs that I'd like to submit to the group for verification. I've made the necessary changes on my end, and I'd be happy to submit them to the development branch.

ISSUE 1

···

---------
This may be specific to the Mac install only. The jar file ${lib}/os-specific/mac/installer-exclude/jmf.jar contains what appears to be duplicate entries of the JMF classes. The duplicate classes appear in a folder of the same name (i.e. JMF) inside the archive.

BUG 1
---------
In the class: net.java.sip.communicator.impl.configuration.ConfigurationServiceImpl the function debugPrintSystemProperties throws a java.util.ConcurrentModificationException caused by the line

for (Map.Entry<Object, Object> entry : System.getProperties().entrySet())

I modified this line and resolved the issue by using a SynchronizedSet wrapper class.

for (Map.Entry<Object, Object> entry : Collections.synchronizedSet(System.getProperties().entrySet()))

I can't say for sure where the error stems from, and I realize that it's evidently a helper function for debugging.

BUG 2
----------
This bug is an issue I discovered when the app attempts to create a LOG folder inside the .SIP-COMMUNICATOR home directory.
The error originates inside net.java.sip.communicator.impl.packetlogging.PacketLoggingServiceImpl :

private void getFileNames()

files[i] = PacketLoggingActivator.getFileAccessService()
                .getPrivatePersistentFile(
                    PacketLoggingActivator.LOGGING_DIR_NAME
                        + File.separator + "jitsi" + i + ".pcap");

But I think the bug exists in net.java.sip.communicator.impl.fileaccess.FileAccessServiceImpl in the function 'accessibleFile'
The function accessibleFile takes 2 parameters. The first is the HOME directory, and the second expects a File name. The function getPrivatePersistentFile, however includes a relative path to the LOG directory (Which is never created), that accessibleFile does not anticipate. This ultimately throws a java.io.FileNotFoundException:
For simplicity the basic fix inside the function 'accessibleFile' was to add the following code towards the end of the function, but I think there were a few other tweaks that I think were needed which are too big to paste into this e-mail.

if (file != null)
{
    file.mkdirs();
}

I hope this proves to be useful.

Thanks.

Oren


#2

Hey Oren,

На 11.05.11 19:33, Oren Forer написа:

Hello all -

I'm fairly new to the Jitsi project so I may lack some background into
the issues I'm about to present so please bare with me. My goal is
focused primarily around Jitsi's SIP capabilities in producing a
(lightweight) java applet that will interface with the browser's UI by
way of javascript (LiveConnect). The applet itself will have no UI
elements, just a basic API that will be called from a JS library.

Sounds interesting! We were also thinking of working along those lines
but haven't got around to it yet.

I've managed to make it work fairly well as a proof of concept, and I
think that speaks volumes to the quality of the Jisti product. So I have
to thank all the contributors.

Our pleasure :wink:

With this background, there are a few issues / bugs that I'd like to
submit to the group for verification. I've made the necessary changes
on my end, and I'd be happy to submit them to the development branch.

ISSUE 1
---------
This may be specific to the Mac install only. The jar file
*${lib}/os-specific/mac/installer-exclude/jmf.jar* contains what appears
to be duplicate entries of the JMF classes. The duplicate classes
appear in a folder of the same name (i.e. JMF) inside the archive.

Oops ... seems like we packaged it once too much. Thanks for the notice!

BUG 1
---------
In the
class: net.java.sip.communicator.impl.configuration.ConfigurationServiceImpl the
function debugPrintSystemProperties throws
a java.util.ConcurrentModificationException caused by the line

for (Map.Entry<Object, Object> entry : System.getProperties().entrySet())

I modified this line and resolved the issue by using a SynchronizedSet
wrapper class.

for (Map.Entry<Object, Object> entry :
Collections.synchronizedSet(System.getProperties().entrySet()))

I can't say for sure where the error stems from, and I realize that it's
evidently a helper function for debugging.

I am wondering how you got that. The method is called in the bundle
start thread, which blocks the entire OSGi initialization so there
shouldn't be anyone playing with the configuration service at that point.

BUG 2
----------
This bug is an issue I discovered when the app attempts to create a LOG
folder inside the .SIP-COMMUNICATOR home directory.
The error originates
inside net.java.sip.communicator.impl.packetlogging.PacketLoggingServiceImpl
:

private void getFileNames()

files[i] = PacketLoggingActivator.getFileAccessService()
                .getPrivatePersistentFile(
                    PacketLoggingActivator.LOGGING_DIR_NAME
                        + File.separator + "jitsi" + i + ".pcap");

But I think the bug exists
in net.java.sip.communicator.impl.fileaccess.FileAccessServiceImpl in
the function 'accessibleFile'
The function accessibleFile takes 2 parameters. The first is the HOME
directory, and the second expects a File name. The
function getPrivatePersistentFile, however includes a relative path to
the LOG directory (Which is never created), that accessibleFile does not
anticipate. This ultimately throws a java.io.FileNotFoundException:

OK I see. I am also curious about this one though. The log directory is
one of the first things we create. How did you end up in a situation
where it wasn't there yet when the packet logger started?

Cheers,
Emil

···

For simplicity the basic fix inside the function 'accessibleFile' was to
add the following code towards the end of the function, but I think
there were a few other tweaks that I think were needed which are too big
to paste into this e-mail.

if (file != null)
{
    file.mkdirs();
}

I hope this proves to be useful.

Thanks.

Oren

--
Emil Ivov, Ph.D. 67000 Strasbourg,
Project Lead France
Jitsi
emcho@jitsi.org PHONE: +33.1.77.62.43.30
http://jitsi.org FAX: +33.1.77.62.47.31


#3

Hi Emil -

Hey Oren,

На 11.05.11 19:33, Oren Forer написа:

Hello all -

I'm fairly new to the Jitsi project so I may lack some background into
the issues I'm about to present so please bare with me. My goal is
focused primarily around Jitsi's SIP capabilities in producing a
(lightweight) java applet that will interface with the browser's UI by
way of javascript (LiveConnect). The applet itself will have no UI
elements, just a basic API that will be called from a JS library.

Sounds interesting! We were also thinking of working along those lines
but haven't got around to it yet.

I've managed to make it work fairly well as a proof of concept, and I
think that speaks volumes to the quality of the Jisti product. So I have
to thank all the contributors.

Our pleasure :wink:

With this background, there are a few issues / bugs that I'd like to
submit to the group for verification. I've made the necessary changes
on my end, and I'd be happy to submit them to the development branch.

ISSUE 1
---------
This may be specific to the Mac install only. The jar file
*${lib}/os-specific/mac/installer-exclude/jmf.jar* contains what appears
to be duplicate entries of the JMF classes. The duplicate classes
appear in a folder of the same name (i.e. JMF) inside the archive.

Oops ... seems like we packaged it once too much. Thanks for the notice!

BUG 1
---------
In the
class: net.java.sip.communicator.impl.configuration.ConfigurationServiceImpl the
function debugPrintSystemProperties throws
a java.util.ConcurrentModificationException caused by the line

for (Map.Entry<Object, Object> entry : System.getProperties().entrySet())

I modified this line and resolved the issue by using a SynchronizedSet
wrapper class.

for (Map.Entry<Object, Object> entry :
Collections.synchronizedSet(System.getProperties().entrySet()))

I can't say for sure where the error stems from, and I realize that it's
evidently a helper function for debugging.

I am wondering how you got that. The method is called in the bundle
start thread, which blocks the entire OSGi initialization so there
shouldn't be anyone playing with the configuration service at that point.

I think I spoke too soon on this issue. Wrapping the Set in a Synchronized Set did not fix this race condition. I can't say with any degree of confidence why the error is thrown. There is likely a separate object that is simultaneously updating the static functions of System Properties. In other words, 2 threads of execution creating their own separate object instances simultaneously accessing the same global / static System properties. The synchronized block will not work as expected using a synchronized block unless the locked object is itself static. I hope I'm not talking out of my ass here. Either way, if an error is thrown, we can ignore it and the Service Should will be created properly.

private void debugPrintSystemProperties()
    {
        if(logger.isInfoEnabled())
        {
            try
            {
                @SuppressWarnings("rawtypes")
                Enumeration e = System.getProperties().keys();
                while (e.hasMoreElements())
                {
                    String key = (String) e.nextElement();
                    logger.info(key + " = " + System.getProperty(key));

                }
                               
                // for (Map.Entry<Object, Object> entry : Collections
                // .synchronizedSet(System.getProperties().entrySet()))
                // logger.info(entry.getKey() + "=" + entry.getValue());
            }
            catch (Exception e)
            {
                // do nothing
            }
        }
    }

BUG 2
----------
This bug is an issue I discovered when the app attempts to create a LOG
folder inside the .SIP-COMMUNICATOR home directory.
The error originates
inside net.java.sip.communicator.impl.packetlogging.PacketLoggingServiceImpl
:

private void getFileNames()

files[i] = PacketLoggingActivator.getFileAccessService()
               .getPrivatePersistentFile(
                   PacketLoggingActivator.LOGGING_DIR_NAME
                       + File.separator + "jitsi" + i + ".pcap");

But I think the bug exists
in net.java.sip.communicator.impl.fileaccess.FileAccessServiceImpl in
the function 'accessibleFile'
The function accessibleFile takes 2 parameters. The first is the HOME
directory, and the second expects a File name. The
function getPrivatePersistentFile, however includes a relative path to
the LOG directory (Which is never created), that accessibleFile does not
anticipate. This ultimately throws a java.io.FileNotFoundException:

OK I see. I am also curious about this one though. The log directory is
one of the first things we create. How did you end up in a situation
where it wasn't there yet when the packet logger started?

My applet is only concerned about the SIP bundle dependencies. I'm not using any of the other non sip features, so I stripped many of the other bundles from the properties file. Perhaps one of those bundles was responsible for insuring the creation of this LOG directory. Either way, here's how I rewrote the function, the patch is attached.

Let me know if there are better ways to contribute code changes.

Thanks.

FileAccessServiceImpl.java.patch (4.54 KB)

···

On May 12, 2011, at 7:52 AM, Emil Ivov wrote:

Cheers,
Emil

For simplicity the basic fix inside the function 'accessibleFile' was to
add the following code towards the end of the function, but I think
there were a few other tweaks that I think were needed which are too big
to paste into this e-mail.

if (file != null)
{
   file.mkdirs();
}

I hope this proves to be useful.

Thanks.

Oren

--
Emil Ivov, Ph.D. 67000 Strasbourg,
Project Lead France
Jitsi
emcho@jitsi.org PHONE: +33.1.77.62.43.30
http://jitsi.org FAX: +33.1.77.62.47.31


#4

На 12.05.11 18:25, Oren Forer написа:

I am wondering how you got that. The method is called in the bundle
start thread, which blocks the entire OSGi initialization so there
shouldn't be anyone playing with the configuration service at that point.

I think I spoke too soon on this issue. Wrapping the Set in a
Synchronized Set did not fix this race condition. I can't say with any
degree of confidence why the error is thrown. There is likely a
separate object that is simultaneously updating the static functions of
System Properties. In other words, 2 threads of execution creating
their own separate object instances simultaneously accessing the same
global / static System properties. The synchronized block will not work
as expected using a synchronized block unless the locked object is
itself static. I hope I'm not talking out of my ass here. Either way,
if an error is thrown, we can ignore it and the Service Should will be
created properly.

I was actually asking how to reproduce the issues from within Jitsi.
What I am gathering from your reply is that they do not actually occur
there and only do in your modified environment.

While it definitely makes sense to have the various services to work in
all possible circumstances, regardless of whether or not they live in
the Jitsi environment, I suspect that issues that arise from such
modifications would likely be regarded as lower priority by most of the
developers in this community.

OK I see. I am also curious about this one though. The log directory is
one of the first things we create. How did you end up in a situation
where it wasn't there yet when the packet logger started?

My applet is only concerned about the SIP bundle dependencies.

In that case, you don't need to care about the packet logger. Just
remove the code that uses it in the SIP bundle.

Cheers,
Emil

···

I'm not
using any of the other non sip features, so I stripped many of the other
bundles from the properties file. Perhaps one of those bundles was
responsible for insuring the creation of this LOG directory. Either way,
here's how I rewrote the function, the patch is attached.

Let me know if there are better ways to contribute code changes.

Thanks.

Cheers,
Emil

For simplicity the basic fix inside the function 'accessibleFile' was to
add the following code towards the end of the function, but I think
there were a few other tweaks that I think were needed which are too big
to paste into this e-mail.

if (file != null)
{
   file.mkdirs();
}

I hope this proves to be useful.

Thanks.

Oren

--
Emil Ivov, Ph.D. 67000 Strasbourg,
Project Lead France
Jitsi
emcho@jitsi.org <mailto:emcho@jitsi.org> PHONE:
+33.1.77.62.43.30
http://jitsi.org FAX: +33.1.77.62.47.31

--
Emil Ivov, Ph.D. 67000 Strasbourg,
Project Lead France
Jitsi
emcho@jitsi.org PHONE: +33.1.77.62.43.30
http://jitsi.org FAX: +33.1.77.62.47.31